-
Notifications
You must be signed in to change notification settings - Fork 21
formal: refactor and unify formality checks #61
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
0a99eb1 to
00aad6f
Compare
00aad6f to
b7803a1
Compare
|
Ping @systemcrash , who has similar thing in LuCI repository. |
|
@GeorgeSapkin you can see my simple if condition here: ( I have no commit rights to the openwrt/actions-shared-workflows repo to amend that workflow ) |
|
Any reason luci has its own formalities (besides Weblate)? I thought they were shared between repos. Other than that, the checks look pretty similar. |
That's a good question, I asked the same while creating openwrt/luci#7821 and openwrt/luci@4dc7a4e#commitcomment-158889550 The goal is indeed to have it shared between all repos, though. 😇
No worries! Just go ahead and submit a PR. It'll be reviewed and merged if everything looks good. |
|
@BKPepe I think it's because weblate is only used by luci so it should be a specific workaround for that. It's ok if we add the workaround here tho :D |
b7803a1 to
20e5ca8
Compare
|
I integrated the Weblate checks from luci feed and extracted the script to a separate file for easier maintainability and debugging. |
7b46c02 to
5c0448f
Compare
.github/scripts/check_formalities.sh
Outdated
| fi | ||
|
|
||
| body="$(git show -s --format=%b $commit)" | ||
| sob="$(git show -s --format='Signed-off-by: %aN <%aE>' $commit)" |
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.
While we're at it, I have a question and I’m not sure if we can address it within this PR.
@Ansuel, @aparcar, what is the policy on using AI to create pull requests and commits?
For example: openwrt/packages#27714
Is this accepted in OpenWrt? These changes are actually not bad and I would merge them, but they can’t pass our checks. :(
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.
@BKPepe that is annoying but a good point... Actually impressed by that pr. Aside from formal and stuff it looks O.K.
Maybe we should discuss them on mailing list.
|
@BKPepe do you think we are ok with merging this? |
BKPepe
left a comment
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.
Otherwise 👍. Thank you! This is really amazing. Hopefully, LuCI repository will use it as well to have really shared workflow.
|
@BKPepe if only github released the paid feature to introduce organization wide workflow... This would have been even easier to implement. |
Formality checks are now unified into a single script that is meant to be shared between all the feeds. Author and Signed-off-by email cannot be a GitHub noreply email as per the contributing guidelines. Add Weblate exceptions for the luci feed. Extract the script to a separate file for easier maintainability and debugging. Signed-off-by: George Sapkin <[email protected]>
5c0448f to
710b121
Compare
Formality checks are now unified into a single script that is meant to be shared between all the feeds.
Author and Signed-off-by email cannot be a GitHub noreply email as per the contributing guidelines. Which is a common issues in luci and packages feeds.
Add Weblate exceptions for the luci feed.
Extract the script to a separate file for easier maintainability and debugging.
Example failing job: https://github.com/GeorgeSapkin/openwrt-packages/actions/runs/19117211025/job/54629195566?pr=3#step:5:7
Example passing job: https://github.com/GeorgeSapkin/openwrt-packages/actions/runs/19117490411/job/54630136784?pr=3#step:5:7
TODO: