GitHub Action and GitLab CI/CD Component#232
Conversation
fc894fd to
089a282
Compare
|
Thanks a lot @gronke for your amazing contributions! |
Installs zpretty from a configurable spec; dogfooded by a workflow.
Add a Continuous integration section to the README with usage examples for the composite action and the GitLab include, and replace the empty 4.0.3 changelog entry in HISTORY.md.
089a282 to
39b2fe0
Compare
|
@ale-rt rebased with a additional comment in the readme, see: |
There was a problem hiding this comment.
Pull request overview
Adds reusable CI integration pieces so downstream projects can easily install and run zpretty in common CI runners (GitHub Actions and GitLab CI/CD), and documents how to use them.
Changes:
- Add a composite GitHub Action for setting up
zpretty, plus a GitHub Actions workflow to smoke-test it. - Add a reusable GitLab CI job template (
.zpretty) and include it in packaging. - Document both CI integrations in
README.mdand note the addition inHISTORY.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Documents GitHub Actions and GitLab CI/CD usage for zpretty setup in CI. |
MANIFEST.in |
Ensures the GitLab CI template YAML is included in source distributions. |
HISTORY.md |
Records the addition of the CI integrations. |
gitlab/zpretty.gitlab-ci.yml |
Provides a reusable .zpretty GitLab job template that installs and reports zpretty. |
.github/workflows/zpretty-action.yml |
Adds a workflow to smoke-test the composite action by installing from the repo checkout and running zpretty --check. |
.github/actions/zpretty/action.yml |
Implements the composite action that sets up Python and installs zpretty from a configurable spec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: actions/checkout@v6 | ||
| - uses: collective/zpretty/.github/actions/zpretty@master | ||
| - run: zpretty --check path/to/file.xml |
There was a problem hiding this comment.
The change of line 147 is wrong.
I agree about the use of <ref> but that should be explained.
There was a problem hiding this comment.
Indeed v6 is the latest Action. One could think that Microsoft trains their models on the docs of their own services 🤔
The benefit of pointing at the default branch @master is that users can just copy-paste and use it. Otherwise one would need to remember what the default branch was called.
@ale-rt think we should change to @<ref>?
There was a problem hiding this comment.
If you look e.g. at https://github.com/actions/checkout#fetch-only-the-root-files they are pointing to a version.
This of course means that once the version is upgraded the docs should be upgraded as well.
Using <ref> and adding to the docs a note about that might be the mvp.
There was a problem hiding this comment.
Ah, I see. You mean to pin the currently latest version in the readme, not <rev>. Makes sense to pin a version, so that linting behavior does not change unexpectedly across repeated builds.
Want to add a drift test for the readme?
|
|
||
| ```yaml | ||
| include: | ||
| - remote: "https://raw.githubusercontent.com/collective/zpretty/master/gitlab/zpretty.gitlab-ci.yml" |
There was a problem hiding this comment.
I agree about the use of but that should be explained.
| before_script: | ||
| - pip install "$ZPRETTY_SPEC" | ||
| - zpretty --version |
There was a problem hiding this comment.
The Python environment should need to bother us here, because if Pip fails installing zpretty, it probably also fails upgrading itself.
| ZPRETTY_PYTHON_VERSION: "3" | ||
| ZPRETTY_SPEC: zpretty | ||
| before_script: | ||
| - pip install "$ZPRETTY_SPEC" |
There was a problem hiding this comment.
The GH action looks like:
python -m pip install --upgrade pip
python -m pip install "${{ inputs.spec }}"
Maybe we should have the same here.
There was a problem hiding this comment.
As mentioned on the Action, I am leaning to remove the pip upgrade, unless you prefer to upgrade it here as well.
| ``` | ||
|
|
||
| The action takes two optional inputs: `spec`, a pip requirement specifier | ||
| (a release such as `zpretty==4.0.2`, a VCS URL, or `.` to install the |
There was a problem hiding this comment.
I do not see the reason of checking out a zpretty and then using a spec different from .
Drop the pip self-upgrade from the composite action: if pip cannot install zpretty it will hardly succeed in upgrading itself first. Use `python -m pip` in the GitLab template too, so both runners install with the identical one-liner.
Replace the hardcoded `master` in the GitHub Action and GitLab include examples with the `4.0.3` release tag, the first one that will ship the action and the template. Copy-pasted pipelines are then pinned, so the formatting behavior does not change unexpectedly between runs; a note explains that any other git reference (a newer tag, a commit SHA, or `master`) works as well. Also clarify that the checkout step in the Action example checks out the consumer repository, not zpretty itself.
Assert that both CI examples pin the same version and that it is either the latest released version or the upcoming one from HISTORY.md. The test stays quiet during a normal development cycle, where the README keeps pointing at the latest release, and fails as soon as the pin falls a full release behind.
for more information, see https://pre-commit.ci
| github, gitlab = self.extract_pinned_versions_from_readme() | ||
| self.assertEqual(github, gitlab) | ||
| topmost, latest_released = self.extract_versions_from_history() | ||
| self.assertIn(github, (topmost, latest_released)) |
There was a problem hiding this comment.
I think this test is a pain to keep it running and not very comprehensive.
As you can see in the README you still have a mixture of 4.0.2 and 4.0.3.
Reusable GitHub Action and GitLab CI/CD Component for zpretty for easy wiring in common CI runners.