Feat: Add pre-commit, on-pr workflow & cr_checker#138
Feat: Add pre-commit, on-pr workflow & cr_checker#138MaximilianSoerenPollak wants to merge 9 commits into
Conversation
This commit adds the cr_checker from the `tools` repo. It works as replacement of the old one from tooling. I also added pre-commit to ensure at least some code quality
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
|
Does this run pre-commit in CI? I dont see it. Consider using central on-pr workflow 😄 |
| @@ -0,0 +1 @@ | |||
| /home/maximilianp/.cache/bazel/_bazel_maximilianp/e21129910acadb4e6a77c10ca6c1c7f2/execroot/_main/bazel-out/k8-fastbuild/bin/.ide_support No newline at end of file | |||
There was a problem hiding this comment.
Do we really need a file in the repo?
There was a problem hiding this comment.
Sorry this should not be in here, I forgot to add this to .gitignore.
|
|
||
| copyright_checker( | ||
| name = "copyright", | ||
| srcs = [ |
There was a problem hiding this comment.
So, we donb't need to specify now explicetly the repo structure or folders to check? the fuill repo will be taken by default?
There was a problem hiding this comment.
Everything that this command git ls-files --cached --other --exclude-standard will find is now in the cr_checker autoomatically :)
| runs: | ||
| using: "docker" | ||
| image: "jorisroovers/gitlint:0.19.1" | ||
| image: "docker://jorisroovers/gitlint:0.19.1" |
There was a problem hiding this comment.
This is needed otherwise it can not infer that the entrypoint should be from the docker and actionlint will give you a warning
| paths: | ||
| .github/workflows/**/*.{yml,yaml}: | ||
| ignore: | ||
| - 'file "/bin/sh" does not exist.*entrypoint' |
There was a problem hiding this comment.
Was the only way I could fix the warning.
It did not accept any other solution, this seems to be accepted workaround (adding the specific thing to the ignore list)
| else | ||
| echo "Could not run '${tool_name}': not available on PATH in a container, and bazel was not found." >&2 | ||
| exit 127 | ||
| fi |
There was a problem hiding this comment.
This file will allow you to run the binaries either from devcontainer or from your system :)
| [tool.basedpyright] | ||
| extends = "bazel-bin/ide_support.runfiles/score_tooling+/python_basics/pyproject.toml" | ||
| verboseOutput = true | ||
| # This file is at the root level, as it applies to all Python code, |
There was a problem hiding this comment.
This is a direct copy of the content of score_tooling+/python_basics/pyproject.toml
You can adapt this as you need it.
The `on-pr` workflow from cicd workflows has all the things we need to execute and has the right permissions.
| common: | ||
| uses: eclipse-score/cicd-workflows/.github/workflows/on-pr.yml@88cb460c47cb49efeb25cd7b73c8945dd1c5bce2 # on-pr/v0.0.0 | ||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
This workflow will start all needed things in the repo, detecting what is inside the repo and then starting it.
There was a problem hiding this comment.
Things like pre-commit for example, that then start other things.
|
@lavrovvalera @AlexanderLanin Please have another look, I addressed the comments and added the missing workflow & removed the not needed file. |
|
I removed basedpyright from the pre-commit config. As it's quiet slow. |
| @@ -29,17 +30,6 @@ docs( | |||
|
|
|||
| copyright_checker( | |||
There was a problem hiding this comment.
This repo now has bazel run //:copyright.fix and pre-commit run which do the same thing. Should we really have both?
There was a problem hiding this comment.
I think for this PR from my side, yes.
I would let the codeowners decide which part they rather would have and delete that one, the PR has both working and shows how to use / implement it.
|
@lavrovvalera according to registry you are the maintainer here, but not a committer. What is going on? More importantly: is someone taking care of it? |
This commit adds the cr_checker from the
toolsrepo, It works as replacement of the old one from tooling.I also added pre-commit to ensure at least some code quality