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

[Github] Do not label clang-format files as clang #66388

Merged
merged 1 commit into from
Sep 16, 2023

Conversation

cor3ntin
Copy link
Contributor

No description provided.

@tstellar
Copy link
Collaborator

This LGTM, but you should get an OK from @AaronBallman

@Endilll
Copy link
Contributor

Endilll commented Sep 15, 2023

This LGTM, but you should get an OK from @AaronBallman

This takes us closer to where we are in bug tracker, i.e. clang label doesn't duplicate clang:* and clang-* labels.
If this gets rejected, I think we're in trouble.

@cor3ntin
Copy link
Contributor Author

@tstellar @Endilll This is mostly a clutch. if we want to support clang:foo not adding clang, I think in the long run we will have to fork the action and adding that ourselves. Expressing everything in terms of negative regex is just way too brittle.

@Endilll
Copy link
Contributor

Endilll commented Sep 15, 2023

Totally agree, but we need to do something quickly, while number of PRs hasn't went out of hand (I see we have about 600-700 PRs a week), and we can fix things retroactively. I'm fine waiting for a better implementation (forking labeler), if there's anyone willing to get to it until the end of next week. Otherwise, I'll prepare brittle exclusion rules today.

I don't want our PRs end in the same sorry state as our bug tracker.

@cor3ntin
Copy link
Contributor Author

I had a chat with Aaron yesterday, he was favorable to merge that, so I'll go ahead.
We had a chat about whether a more general solution was needed, and if so we can implement something better than negative matches, but this require a longer discussion (ad quite a bit more work) and we might not have consensus to do so. I wrote up my thoughts here https://discourse.llvm.org/t/hows-it-going-with-pull-requests/73467/27?u=cor3ntin

@cor3ntin cor3ntin merged commit f3796ac into llvm:main Sep 16, 2023
2 checks passed
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants