-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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
feat(CI): Added GitHub Actions to check for code style violations #13489
Conversation
/format |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.2 #13489 +/- ##
==========================================
- Coverage 70.38% 70.35% -0.04%
==========================================
Files 1606 1606
Lines 69999 69999
Branches 10098 10098
==========================================
- Hits 49272 49248 -24
- Misses 16103 16115 +12
- Partials 4624 4636 +12 ☔ View full report in Codecov by Sentry. |
@AlbumenJ I have added a slight tab to break the style of one of the java file to check if the CI actually works. |
@AlbumenJ I think the workflows don't have read and write permission. You can go to |
Seems it should be changed to scheduled task to scan PRs due to permission limitation. The ASF will not enable write access in PRs due to workflows file can be changed by anyone. So it might be a good idea to switch to other kinds of task. |
@AlbumenJ Okay, I see. So, do you mean that this workflow should run on schedule and will just check if codestyle is followed? |
Yes :) |
@AlbumenJ Okay. Then, what should be the time of the workflow run in UTC timezone (this timezone is followed by GH Actions)? Should I keep it at midnight UTC everyday? You can check and give me a custom cron time from https://crontab.guru/ . |
Every hour or even 20 mins are both ok |
Okay @AlbumenJ |
@AlbumenJ I have added the cron job in the workflow to run at every 20 mins. |
@AlbumenJ The cron job worked 🎉 - you can check here - https://github.com/SaptarshiSarkar12/dubbo/actions/runs/7330812949 |
@SaptarshiSarkar12 I have an individual suggestion: add a ci code checkstyle to each submission instead of completing it through schedule. |
@CrazyHZM Okay. But, I couldn't understand your suggestion. Can you please elaborate? |
@AlbumenJ Should I add the Check for formatting workflow to Build and Test Schedule also? |
I think it can be omitted. Because the scheduled task itself will check the format. |
@AlbumenJ Should the workflow remain in the Build and test PR workflow? I couldn't understand your words. Can you please elaborate? |
Sorry about that. The workflow of
|
@AlbumenJ It's okay. |
@AlbumenJ Please review this PR then. Let me know if any further changes are required. |
@AlbumenJ I don't think the scheduled check creates summary if linting fails. Can I try adding it? |
This PR LGTM |
|
Okay @AlbumenJ. So, can I remove my changes which were made solely for testing the CI (like adding extra spaces)? |
Yep |
Okay @AlbumenJ. Would do it soon. |
@AlbumenJ I have made the required changes. |
|
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.
LGTM
@CrazyHZM PTAL |
@SaptarshiSarkar12 Thanks for your contribution :) |
|
Okay. Got it. Thank you @liaozan 😄! |
What is the purpose of the change
Closes #13413
Brief changelog
Checklist