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

CI: Add the label-check workflow to verify changelog labels on each PR #4847

Merged
merged 8 commits into from Oct 16, 2023

Conversation

anshgoyalevil
Copy link
Contributor

@anshgoyalevil anshgoyalevil commented Oct 16, 2023

Which problem is this PR solving?

Description of the changes

  • This workflow would first check if the PR is created by @dependabot or not. If no, it would proceed to check for the labels on that pull request. For example:
  • A new PR is created. The Label Check workflow would fail
  • A maintainer adds some label bug on the PR, workflow would again run, but would still fail since the label bug doesn't start with changelog:
  • The maintainer now adds some label changelog:**, this time, Label Check CI would pass.

How was this change tested?

How to test this PR?

  • Play with the labels by applying or removing them and see the Verify Label action fail/pass.

Checklist

Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
@anshgoyalevil anshgoyalevil requested a review from a team as a code owner October 16, 2023 09:45
Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified lines are covered by tests ✅

see 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
@yurishkuro
Copy link
Member

closed?

@anshgoyalevil
Copy link
Contributor Author

This workflow was working correctly but later I realized that I couldn't solve the complete issue of modifying the release scripts as I don't know Python :(

@anshgoyalevil
Copy link
Contributor Author

anshgoyalevil commented Oct 16, 2023

I am currently trying out the modification to the Python script as I learn it. In the meantime, others can try their own approach. Merging this PR would have constrained me to resolve the other part of the issue quickly, as no other folk would then try out their hand thereafter.

@yurishkuro
Copy link
Member

yurishkuro commented Oct 16, 2023

Having a check on PRs before merging is an independent of whether we have a release script making use of that.

There seems to be in-progress PR for the latter #4849

@anshgoyalevil anshgoyalevil reopened this Oct 16, 2023
@anshgoyalevil
Copy link
Contributor Author

anshgoyalevil commented Oct 16, 2023

I managed to modify the python script also, but #4849 seems to have a better approach.

Feel free to play with the PR by adding/removing labels and verify if the action is working properly

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Oct 16, 2023
@yurishkuro
Copy link
Member

image

@yurishkuro yurishkuro merged commit 8727aba into jaegertracing:main Oct 16, 2023
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:ci Change related to continuous integration / testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants