Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve display of build targets #262

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

trz42
Copy link
Contributor

@trz42 trz42 commented Mar 13, 2024

Shortens the list of "build targets" (architectures and repository ids). This is shown when a PR is opened or for the command bot: show_config. Tested in trz42/software-layer#72

Example:

Old display

Instance dev-PR254 is configured to build:

  • arch x86_64/amd/zen2 for repo nessi-2022.11-swl-deb10
  • arch x86_64/amd/zen2 for repo nessi-2023.06-cl
  • arch x86_64/amd/zen2 for repo nessi-2023.06-swl-deb10
  • arch x86_64/amd/zen2 for repo nessi-2023.06-swl-deb11
  • arch aarch64/generic for repo nessi-2022.11-swl-deb10
  • arch aarch64/generic for repo nessi-2023.06-cl
  • arch aarch64/generic for repo nessi-2023.06-swl-deb10
  • arch aarch64/generic for repo nessi-2023.06-swl-deb11
  • arch aarch64/thunderx2 for repo nessi-2022.11-swl-deb10
  • arch aarch64/thunderx2 for repo nessi-2023.06-cl
  • arch aarch64/thunderx2 for repo nessi-2023.06-swl-deb10
  • arch aarch64/thunderx2 for repo nessi-2023.06-swl-deb11

New display

Instance dev-PR254 is configured to build for:

  • architectures: x86_64/amd/zen2, aarch64/generic, aarch64/thunderx2
  • repositories: nessi-2023.06-cl, nessi-2023.06-swl-deb11, nessi-2023.06-swl-deb10, nessi-2022.11-swl-deb10

Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and a really nice improvement given the number of architectures and repos will likely keep increasing! 🥳

No major comments or things to be changed on the code but something to think about for the future:

Most instances will be running on sites where the same architectures are relevant for all repos. This doesn't necessarily have to be the case (e.g. map of zen2 and zen3 for repo A and only zen2 in repo B) and in those cases that distinction is lost with the new reporting (unless I'm missing something). This new conciseness is much better than the previous bullet list, but maybe in a future iteration it might be nice to use a "drop-down on click" approach, or have something like

  • zen3 - repoA, repoB
  • zen2 - repoB

@trz42
Copy link
Contributor Author

trz42 commented Mar 14, 2024

Most instances will be running on sites where the same architectures are relevant for all repos. This doesn't necessarily have to be the case (e.g. map of zen2 and zen3 for repo A and only zen2 in repo B) and in those cases that distinction is lost with the new reporting (unless I'm missing something). This new conciseness is much better than the previous bullet list, but maybe in a future iteration it might be nice to use a "drop-down on click" approach, or have something like

  • zen3 - repoA, repoB
  • zen2 - repoB

That's a nice idea. (Thought about that too, but then was too lazy to implement it in the first iteration.)

@boegel
Copy link
Contributor

boegel commented Mar 14, 2024

I agree with Pedro's feedback, why not simply do something like:

Instance dev-PR254 is configured to build:

<details>
...
</details>

@Neves-P Neves-P merged commit 2d15e77 into EESSI:develop Mar 14, 2024
7 checks passed
@trz42
Copy link
Contributor Author

trz42 commented Mar 14, 2024

I agree with Pedro's feedback, why not simply do something like:

Instance dev-PR254 is configured to build:

<details>
...
</details>

Sure. Another option would be

Instance dev-PR254 is configured to build: (click for details)
  • zen3 - repoA, repoB
  • zen2 - repoB

Little downside is that you have to click to see it.

Anyhow everyone can make a PR for an improvement.

@trz42 trz42 mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants