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.

Sign up for GitHub

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

Jump to bottom

Prettify labels in action progress messages with Bzlmod #17045

Closed
wants to merge 1 commit into from
Closed

Prettify labels in action progress messages with Bzlmod #17045

wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 17, 2022 •
edited
Loading

UiStateTracker is provided with the repository mapping of the main repository after the loading phase has been completed and uses this mapping to "unmap" canonical labels back to the apparent name used for them by the main repository.

Fixes #17130

@fmeum fmeum changed the title Prettify labels in execution phase progress messages WIP: Prettify labels in execution phase progress messages Dec 17, 2022
@fmeum fmeum force-pushed the prettify-progress-labels branch 4 times, most recently from b9f90ff to 2fc85c0 Compare December 18, 2022 07:46
@fmeum fmeum changed the title WIP: Prettify labels in execution phase progress messages Prettify labels in action progress messages with Bzlmod Dec 18, 2022
@fmeum fmeum marked this pull request as ready for review December 18, 2022 10:46
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 18, 2022

@Wyverald Could you review this PR?

@joeleba I have trouble getting UiStateTrackerTest to pass in the Skymeld case. This change makes it necessary to fire a LoadingPhaseCompleteEvent, but that seems to affect the SkymeldUiStateTracker state in a way I don't understand. Could you help with that?

@sgowroji sgowroji added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Dec 18, 2022
@joeleba
Copy link
Member

joeleba commented Dec 20, 2022

@zhengwei143 Could you please help with the failing test? Thanks!

@zhengwei143
Copy link
Contributor

Seems like you just need to set mainRepositoryMapping = event.getMainRepositoryMapping(); in the SkymeldUiStateTracker#loadingComplete method. SkymeldUiStateTracker overrides that method - so the repository mapping wasn't set for the Skymeld case and hence you got a lot of NullPointerException(s) and failing tests.

@fmeum
Prettify labels in action progress messages with Bzlmod
e106a2d 
`UiStateTracker` is provided with the repository mapping of the main
repository after the loading phase has been completed and uses this
mapping to "unmap" canonical labels back to the apparent name used for
them by the main repository.
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 20, 2022

@zhengwei143 Thanks, that seems to have worked, tests are passing now.

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 3, 2023

@Wyverald Friendly ping

@sgowroji sgowroji added the awaiting-review PR is awaiting review from an assigned reviewer label Jan 3, 2023
@fmeum fmeum mentioned this pull request Jan 4, 2023
Closed
Wyverald
Wyverald approved these changes Jan 4, 2023
View reviewed changes
Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

Thanks, and sorry for the late review!

@Wyverald Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 4, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 4, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 4, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 11, 2023
@fmeum fmeum deleted the prettify-progress-labels branch January 11, 2023 13:48
@ShreeM01
Copy link
Contributor

@bazel-io fork 6.1.0

@bazel-io bazel-io mentioned this pull request Jan 19, 2023
Closed
@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 19, 2023
@ShreeM01 ShreeM01 mentioned this pull request Jan 20, 2023
Merged
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
@fmeum @hvadehra
Prettify labels in action progress messages with Bzlmod
151e1a4 
`UiStateTracker` is provided with the repository mapping of the main repository after the loading phase has been completed and uses this mapping to "unmap" canonical labels back to the apparent name used for them by the main repository.

Fixes #17130

Closes #17045.

PiperOrigin-RevId: 501256229
Change-Id: I796585da07773ee3a589bb83948b9e0667b2e799
ShreeM01 added a commit that referenced this pull request Feb 14, 2023
@ShreeM01 @fmeum
Prettify labels in action progress messages with Bzlmod (#17278)
4e35c02 
`UiStateTracker` is provided with the repository mapping of the main repository after the loading phase has been completed and uses this mapping to "unmap" canonical labels back to the apparent name used for them by the main repository.

Fixes #17130

Closes #17045.

PiperOrigin-RevId: 501256229
Change-Id: I796585da07773ee3a589bb83948b9e0667b2e799

Co-authored-by: Fabian Meumertzheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wyverald Wyverald Wyverald approved these changes

Assignees
No one assigned
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cleanup progress when using bzlmod
7 participants
@fmeum @joeleba @zhengwei143 @ShreeM01 @Wyverald @bazel-io @sgowroji