Skip to content

[5.4] Duplicate error message on package resolve #3258

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
Merged
merged 1 commit into from
Feb 12, 2021
Merged

[5.4] Duplicate error message on package resolve #3258

merged 1 commit into from
Feb 12, 2021

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Feb 12, 2021 •
edited
Loading

motivation: fix a bug where in some cases a duplicate error message is printed

changes: only try to load the graph if the previous operation succeeded

@tomerd
Duplicate error message on package resolved
6184b4d 
motivation: fix a bug where in some cases a duplicate error message is printed

changes: only try to load the graph if the previous operation succeeded
@tomerd tomerd added the 5.4 label Feb 12, 2021
@tomerd
Copy link
Contributor Author

tomerd commented Feb 12, 2021

@swift-ci please test

abertelrud
abertelrud approved these changes Feb 12, 2021
View reviewed changes
Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

This seems like a safe fix for release/5.4, though as noted in the main PR, it seems that we might still be susceptible to double warnings here. It might be worth reworking the code in main, though it's hardly the most important issue unless we're seeing double warnings today.

@tomerd
Copy link
Contributor Author

tomerd commented Feb 12, 2021

@swift-ci please test linux

@tomerd
Copy link
Contributor Author

tomerd commented Feb 12, 2021

looks like testBrokenRefresh is still flaky, need to look into that again

@tomerd
Copy link
Contributor Author

tomerd commented Feb 12, 2021 •
edited
Loading

This seems like a safe fix for release/5.4, though as noted in the main PR, it seems that we might still be susceptible to double warnings here. It might be worth reworking the code in main, though it's hardly the most important issue unless we're seeing double warnings today.

agreed, I think we should put this into 5.4 and discuss what better ways we have in #3257

@tomerd tomerd changed the title Duplicate error message on package resolved [5.4] Duplicate error message on package resolve Feb 12, 2021
@tomerd tomerd self-assigned this Feb 12, 2021
@tomerd
Copy link
Contributor Author

tomerd commented Feb 12, 2021

@swift-ci please test linux

@tomerd tomerd merged commit 609d57e into swiftlang:release/5.4 Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abertelrud abertelrud abertelrud approved these changes

@friedbunny friedbunny Awaiting requested review from friedbunny friedbunny is a code owner

@neonichu neonichu Awaiting requested review from neonichu neonichu is a code owner

Assignees

@tomerd tomerd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomerd @abertelrud