-
Notifications
You must be signed in to change notification settings - Fork 11
Update for gitlint, docs and rules_python #28
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
Use the base-branch provided as input for the gitlint call.
Update score_docs_as_code to 0.3.3 and adapted usage in according workflow.
Update rules_python module to 1.4.1
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: ✅ Passed Click to expand output |
No need for overwrite defaults to origin/main which is for now the main use case. Still variable could be used if needed.
| - -c | ||
| - | | ||
| git config --global --add safe.directory /github/workspace && \ | ||
| git fetch origin +refs/heads/main:refs/remotes/origin/main && \ |
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.
you also need to fetch that base branch.
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.
@nicu1989 why don't we use something generic like this instead of input variables?
TARGET_BRANCH="${{ github.event.pull_request.base.ref || github.ref_name }}"
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 action cannot access to the github.* context at execution time
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.
Ah I see, because it's a docker action.
So as long as the default works we can keep it as is.
But if we need better support of e.g. auto detecting branches, then we should convert this to a composite action?!
(or javascript action if someone is bored)
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.
yeah composite action should work with github context. my remark was to update the fetch as well since the author changed the gitlint command from default main to input.base-branch
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'll merge this now as the PR is an improvement, even if incomplete
update various bazel modules and usage