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

Correctly iterate IComputers from ComputerSet/index.jelly #9852

Merged
merged 1 commit into from
Oct 12, 2024

Conversation

jglick
Copy link
Member

@jglick jglick commented Oct 11, 2024

#9749 updates the executors widget to display the new list of IComputers, but the nodes overview pages was forgotten. get_all only returns the old list of Computers.

Testing done

Tested interactively and with an automated test in CloudBees CI.

Proposed changelog entries

  • N/A

Proposed upgrade guidelines

N/A

Before the changes are marked as ready-for-merge:

Maintainer checklist

@jglick jglick requested a review from Vlatombe October 11, 2024 01:21
@MarkEWaite MarkEWaite added bug For changelog: Minor bug. Will be listed after features skip-changelog Should not be shown in the changelog and removed bug For changelog: Minor bug. Will be listed after features labels Oct 11, 2024
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks! I assume this is skip-changelog because the issue is not visible to a typical Jenkins user. I was unable to see any issue on my installation of Jenkins 2.480 when viewing the list at /computer/.

@MarkEWaite
Copy link
Contributor

@jglick, thanks for the pull request. I've inserted the "Proposed upgrade guidelines" section so that the changelog generator will provide the correct changelog entry from the pull request description. If you could retain that section in future pull requests, it would avoid me needing to edit the changelog description.

@Vlatombe
Copy link
Member

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Oct 11, 2024
@jglick
Copy link
Member Author

jglick commented Oct 11, 2024

I assume this is skip-changelog because the issue is not visible to a typical Jenkins user.

Correct, this can only affect CloudBees CI (in HA mode) where there are IComputers which are not instanceof Computer.

I've inserted the "Proposed upgrade guidelines" section so that the changelog generator will provide the correct changelog entry from the pull request description. If you could retain that section in future pull requests

Oops, sorry, was not aware.

@timja
Copy link
Member

timja commented Oct 12, 2024

I've inserted the "Proposed upgrade guidelines" section so that the changelog generator will provide the correct changelog entry from the pull request description. If you could retain that section in future pull requests

Oops, sorry, was not aware.

I don't think it will matter for a skip-changelog but in general leave all sections of the PR template in place 👍 .

@MarkEWaite MarkEWaite merged commit a1fc71e into jenkinsci:master Oct 12, 2024
15 checks passed
@jglick jglick deleted the ComputerSet branch October 13, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback skip-changelog Should not be shown in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants