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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

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
Co-authored-by: Lily Wang <94091114+LilyWangLL@users.noreply.github.com>
@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.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants