Skip to content
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

Reworks workflows to integrate release creation and trusted publishing in addition to testing and linting #35

Merged
merged 8 commits into from
Sep 24, 2024

Conversation

stumpylog
Copy link
Contributor

@stumpylog stumpylog commented Sep 20, 2024

Description

I always find the box view best:

Running against a normal commit: https://github.com/stumpylog/ServeStatic/actions/runs/10951878142

When run with a tag: https://github.com/stumpylog/ServeStatic/actions/runs/10951805566

An example release: https://github.com/stumpylog/ServeStatic/releases

basically, linting first, the testing the code, updating documentation, and if triggered via tag, creating a release and publishing to pypi. Some jobs could be combined, such as documentation being a single job which lints, build and deploys according to the Git ref.

Closes #34

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
with:
merge-multiple: true

- name: Combine coverage and fail if it's <95%
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Around here would be a great place to integrate codecov if ever desired

@stumpylog
Copy link
Contributor Author

Seems like the pre-commit bot ran and changed a bunch of files. Maybe a little early?

@Archmonger
Copy link
Owner

Yeah - probably should have a dedicated PR just to let precommit muck around with all the files.

I can do that tomorrow, or you can do a precommit CI PR it and I'll merge.

@stumpylog stumpylog marked this pull request as ready for review September 21, 2024 16:37
Copy link
Owner

@Archmonger Archmonger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently not home so this is a half-review I quickly did on mobile. Will do a proper review tomorrow.

.editorconfig Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@Archmonger
Copy link
Owner

Also as a note, I only have squash merge turned on for any repos I manage, so you can save yourself time and not worry about commit history if you'd like.

@stumpylog
Copy link
Contributor Author

Not really sure that the link check thing is. Seems alright, except for all the resource warnings.

@Archmonger
Copy link
Owner

Archmonger commented Sep 23, 2024

Dang, that link check error is ReadTheDocs refusing to process HTTP requests originating from GitHub workflows.

This just occurred within the past day, but it looks like they got annoyed with the sheer amount of HTTP requests coming from workflows...

I'm tempted to say we wait a day and see if they backtrack on their decision. Otherwise, we might be forced to remove linkcheckmd from the workflow.

Maybe we can switch to linkspector and add our ReadTheDocs links to the ignorePatterns?

@stumpylog
Copy link
Contributor Author

Since it's not related to this pr, I'd prefer to keep it separate and merge into this. I probably won't have time for a bit to experiment

@Archmonger
Copy link
Owner

Archmonger commented Sep 24, 2024

I just experimented with it on another repo and linkspector is broken at the moment. We can just disable linkcheckmd temporarily.

@Archmonger Archmonger linked an issue Sep 24, 2024 that may be closed by this pull request
@Archmonger Archmonger merged commit f18f78f into Archmonger:main Sep 24, 2024
16 checks passed
@Archmonger
Copy link
Owner

@stumpylog In terms of this new workflow, what is your proposed release process?

This CI doesn't appear to have any events related to tag creation, so startsWith(github.ref, 'refs/tags/') is never true.

I've tried manually tagging a commit in main then re-running the workflow but it still does not publish properly.

@stumpylog
Copy link
Contributor Author

On.push should be trigger by the pushing of a new tag. I can look at the co runs tomorrow from a computer

@Archmonger
Copy link
Owner

Archmonger commented Oct 3, 2024

on > push currently doesn't appear to occur on tag pushes.

I just tried a workaround that uses the on > release trigger hoping that the workflow would override the existing release, Unfortunately, the workflow doesn't want to override existing releases, despite the workflow's github indicating that it should be able to.

Perhaps we need to add a tags value to the on > push trigger

on:
  push:
    branches:
      - main
    tags:
      - "**"

@Archmonger
Copy link
Owner

Archmonger commented Oct 3, 2024

Looks like there's a allowsUpdates parameter I had forgotten to set in my last attempt at fixing this. New PR created.

I think either a tags: ** or override on: publish approach should work. I'm leaning towards just creating blank releases on GitHub and having on: publish handle the rest.

@stumpylog
Copy link
Contributor Author

These precise conditions works fine for me: https://github.com/stumpylog/gotenberg-client/blob/3ee137c559beffb9239b9dbe28ca4c1b0ca4b872/.github/workflows/ci.yml#L3-L8 and then https://github.com/stumpylog/gotenberg-client/blob/3ee137c559beffb9239b9dbe28ca4c1b0ca4b872/.github/workflows/ci.yml#L228-L231

I'm not sure what it could be. The name of the ref is correct. How are you creating it? Just git tag 2.1.0 and git push origin 2.1.0?

@Archmonger
Copy link
Owner

I believe I had run git tag 2.1.0 then git push --tags

@stumpylog
Copy link
Contributor Author

Looking at the two runs which were named 2.1.0, both were triggered via a pull request from a branch.

Currently, with on.push.branches set, the workflow will not run when a tag is pushed. See here. You would need to explicitly add tags: now. Otherwise "If you define neither tags/tags-ignore or branches/branches-ignore, the workflow will run for events affecting either branches or tags"

@Archmonger
Copy link
Owner

Archmonger commented Oct 3, 2024

Makes sense. I needed to specify a branch to avoid getting double-spammed with CI runs from my PRs.

I'll need to manually add a on > push > tags in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add editor configuration files Automate GitHub release workflow
2 participants