-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
adding the script to validate changelog #4731
adding the script to validate changelog #4731
Conversation
Thank you for your contribution! 🙏 We will review your PR as soon as possible. While you are waiting, make sure to:
Learn more about: |
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.
Thanks for this! The changelog is sorted, but we also tend to put **General**:
before all other values.
Also would be nice to add a way to skip the Changelog validation for a certain PR. (eg. we have the skip-e2e
label for skipping e2e test check).
I'm not sure if we should be able to skip Changelog validation, it's more like other CI checks than e2e test IMO, we cannot skip static checks (and this is one static check more). Personally, I'm fine without skipping it, but I prefer to execute it as part of pre-commit if it's possible. If somebody is using pre-commit, it'll cover this new check automatically WDYT? |
Good point! |
hey @JorTurFer @zroubalik Please take a look. |
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
22ab358
to
59095f8
Compare
@zroubalik To my understanding as there are other values as well checking for General specifically will not give us any result. As we than also need to allow for other **example name** also. If we can limit those names than that check would be a good fit. WDYT? |
3abe79c
to
1dca47f
Compare
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
1dca47f
to
2bb5f47
Compare
General is the only value that has to be prioritized. This is because usually General affects to all the users and that's why we place it on top of all the sections. The style rules are here: https://github.com/kedacore/keda/blob/main/CONTRIBUTING.md#Changelog Said that, maybe we can add 2 steps in the |
@JorTurFer sounds great, i will add it. |
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.
This is nice! My only question is if we can check all the changelog and not just unreleased.
I say this because during cherry-picks for hotfix releases we can make mistakes
we can but we need to change all previous to correct order as well first. Some also have entries missing as well. Like having change name only and doesn't having pr reference. |
If we can, it'd be nice. The changelog has been chaotic in the past 😞 |
I will try. |
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
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.
This looks nice!
Let me check it with some cases locally and that'd be all I forgot the point of all the versions
Please, remember to undo the change in pr-validation.yaml
hey @JorTurFer I have tried the script to validate the change in each version. The script is:
But the problem is, some changes are not logged properly, and not easy to search for in the pr and issues, may be they are named differently there.
|
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
f6bb0cf
to
894e194
Compare
Let me take a look, we haven't been following the same rules since the beginning, let me check it changelog for trying to limit the scope 😄 |
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
There are a lot of issues on the changelog T.T |
Based on your script, I have done some small modifications (to exclude v1.* and to support multiple repetitions of the pattern) and I have fixed all the issues on the changelog. After that, it works really nice ❤️ As I said, if you agree, I can commit the changes to your branch directly |
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
@JorTurFer yes please. Thankyou |
Done! |
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
/skip-e2e |
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Thanks for your kind words, It looks good to me. |
Why is failing in this: |
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
That failure that the commit solves is related with missing new line at the end of the file. It's this rule: https://github.com/kedacore/keda/blob/main/.pre-commit-config.yaml#L16C7-L16 About why the script that validates the changelog failed, it looks related with some small differences between My latest commit updates some lines to be explicit: 2739bb1 Let's see 🤞 |
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
@zroubalik PTAL |
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Co-authored-by: Jorge Turrado <jorge_turrado@hotmail.es> Co-authored-by: Jorge Turrado <jorge.turrado@scrm.lidl> Signed-off-by: Yoon Park <yoongon.park@gmail.com>
Co-authored-by: Jorge Turrado <jorge_turrado@hotmail.es> Co-authored-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Co-authored-by: Jorge Turrado <jorge_turrado@hotmail.es> Co-authored-by: Jorge Turrado <jorge.turrado@scrm.lidl>
adding the script to validate changelog by make validate-changelog and updating github action
Checklist
Fixes #3190