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

Minor updates to comments for some verbiage that gets flagged by political correctness tools #38719

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

ShiningMassXAcc
Copy link
Member

Replaces two terms used in comments with some more politically correct alternatives.

@LilyWangLL LilyWangLL added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines labels May 14, 2024
@LilyWangLL
Copy link
Contributor

Just modify the comments, so I canceled the CI pipeline run.

@dg0yt
Copy link
Contributor

dg0yt commented May 14, 2024

Not ready for merge.

Just modify the comments, so I canceled the CI pipeline run.

Bad idea. CI would catch the missing port version bump for qtbase which the reviewer didn't notice.
And CI build time would show the price the change has for vcpkg user.

@dg0yt
Copy link
Contributor

dg0yt commented May 14, 2024

And please don't delay the other qtbase PRs over it!

@dg0yt
Copy link
Contributor

dg0yt commented May 14, 2024

The qtbase change was cherry-picked into #38682.

@LilyWangLL LilyWangLL removed the info:reviewed Pull Request changes follow basic guidelines label May 14, 2024
@LilyWangLL
Copy link
Contributor

Note: I will be converting your PR to draft status. The above suggested changes are only recommendations. If you are willing to adopt them, please click "ready for review" after making the modifications. If you do not wish to make any changes, please click "ready for review" directly. That way, I can be aware that you've responded since you can't modify the tags.

@LilyWangLL LilyWangLL marked this pull request as draft May 14, 2024 06:10
@ShiningMassXAcc
Copy link
Member Author

Note: I will be converting your PR to draft status. The above suggested changes are only recommendations. If you are willing to adopt them, please click "ready for review" after making the modifications. If you do not wish to make any changes, please click "ready for review" directly. That way, I can be aware that you've responded since you can't modify the tags.

Thanks (I was out for a bit, so coming back here very late)!

@ShiningMassXAcc ShiningMassXAcc marked this pull request as ready for review May 24, 2024 16:52
@BillyONeal
Copy link
Member

I cancelled this because it was going to take machine-days of time and invalidate all vcpkg customers' caches, and I don't think that's worth it to fix a comment. However, next time vcpkg_configure_make needs to be changed for any other reason I think this should be included into it.

@BillyONeal BillyONeal merged commit 027b48d into microsoft:master Jun 14, 2024
6 of 17 checks passed
@BillyONeal
Copy link
Member

I merged this because I just merged another change which invalidated basically all caches. I didn't retrigger the effective world rebuild from it because it only changes a comment.

@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants