-
Notifications
You must be signed in to change notification settings - Fork 1.8k
CI: Add Kotlin and Go to labeler.yml #9396
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
Conversation
@@ -6,22 +6,32 @@ | |||
- csharp/**/* | |||
- change-notes/**/*csharp* | |||
|
|||
Go: | |||
- go/**/* | |||
- change-notes/**/*go.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the other languages, I think this should be:
- change-notes/**/*go.* | |
- change-notes/**/*go* |
Although it looks like for Java we use .*
as well. This is probably to avoid overlap with javascript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch that, we should just drop this pattern. The legacy Go change notes are in go/old-change-notes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the .
matters much either way, except for java where we need it. It might be better to always include it.
I imagine that future go changenotes will follow the same pattern as the other languages now that go is in this repo.
.github/labeler.yml
Outdated
- java/kotlin-extractor/**/* | ||
- java/kotlin-explorer/**/* | ||
- java/ql/test/kotlin/**/* | ||
- change-notes/**/*java.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably just drop this line. I think the change-notes
folder is no longer used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want the Kotlin label for Java related change notes, then you need the following pattern java/ql/*/change-notes
. I think this is probably a bad idea though. If there is a kotlin related QL change note then there should probably also a change in the kotlin specific QL tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed it from Kotlin.
I'll leave looking at whether the changenotes patterns should generally be updated for a future PR.
@@ -6,22 +6,32 @@ | |||
- csharp/**/* | |||
- change-notes/**/*csharp* | |||
|
|||
Go: | |||
- go/**/* | |||
- change-notes/**/*go.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch that, we should just drop this pattern. The legacy Go change notes are in go/old-change-notes
.
They get labeled as Java. Given we aren't labeling shared QLL changes, it makes sense not to label shared changenotes either.
No description provided.