Skip to content

fix: auto release on mirror updates #83

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.

Sign up for GitHub

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

Merged
zanieb merged 2 commits into from
Apr 4, 2024
+27 −21
Merged

fix: auto release on mirror updates #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b53f168
fix: auto release on mirror updates
CoderJoshDK Apr 3, 2024
1d4627a
build: add names to commands for clarity
CoderJoshDK Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,32 @@ jobs:

- run: python mirror.py

- run: |
- name: check for unpushed commits
id: check_unpushed
run: |
UNPUSHED_COMMITS=$(git log origin/main..HEAD)
if [ -z "$UNPUSHED_COMMITS" ]; then
echo "No unpushed commits found."
echo "changes_exist=false" >> $GITHUB_ENV
else
echo "Unpushed commits found."
echo "changes_exist=true" >> $GITHUB_ENV
fi
Comment on lines +35 to +45
Copy link
Copy Markdown
Member

@zanieb zanieb Apr 3, 2024

Choose a reason for hiding this comment

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

This seems fine but I wonder if mirror.py should indicate if this needs to happen or not? Perhaps for tracking in a follow-up

Copy link
Copy Markdown
Contributor Author

@CoderJoshDK CoderJoshDK Apr 3, 2024

Choose a reason for hiding this comment

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

The created variable allows for any extra function to be added based on $changes_exist. Making mirror.py send out some type of signal doesn't make the most sense to me. But maybe you have something specific in mind?

Copy link
Copy Markdown
Member

@zanieb zanieb Apr 3, 2024

Choose a reason for hiding this comment

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

Well it could be as simple as "echo "changes_exist=$(python mirror.py)" >> $GITHUB_ENV" and having mirror.py emit true or false to STDOUT. The idea here is that mirror.py would say if you need to publish a release instead of checking for extra commits and releasing based on a side-effect which is more likely to be error prone.

Copy link
Copy Markdown
Contributor Author

@CoderJoshDK CoderJoshDK Apr 3, 2024

Choose a reason for hiding this comment

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

I think I figured out a way to make this work. However, it is dependent on side effects. So let us say we changed mirror.py to look like:

https://github.com/CoderJoshDK/ruff-pre-commit/blob/a628536dcda0210e9733cf7056947769fc58e4b4/mirror.py#L12-L25

And then changed main.yml to look like:

https://github.com/CoderJoshDK/ruff-pre-commit/blob/a628536dcda0210e9733cf7056947769fc58e4b4/.github/workflows/main.yml#L33-L42

It would roughly do what you want. However, there are multiple issues with this approach. It works (should, will make sure to do more testing if you are ok with this issue) but only because of the side effect where the subprocess in mirror writes to STDOUT. And so now we end up trading one side effect for another. If you still feel that this is cleaner, that is fine, and I can put these changes in.
Example output:

❯ echo $(python3 mirror.py)
[test f00a01e] Mirror: 0.3.5 1 file changed, 1 insertion(+), 1 deletion(-)


❯ echo $(python3 mirror.py)

And if you do want this change, do you want it as a separate PR?

Copy link
Copy Markdown
Member

@zanieb zanieb Apr 4, 2024

Choose a reason for hiding this comment

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

Ah I would not depend on the subprocess writing to stdout that's too confusing. Let's just consider this separately there's no reason to slow down this fix. I probably ought to actually look at it myself.

Copy link
Copy Markdown
Contributor Author

@CoderJoshDK CoderJoshDK Apr 4, 2024

Choose a reason for hiding this comment

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

You could always have it print out some random value when a commit is made, just to double make sure a write to the STDOUT is done. But the code ends up being exactly the same with an extra print.

Personally, I feel like the current state of things is easier to follow. Being too cleaver might not actually make anything better. Regardless, I am interested to see if you manage to think of a solution.


- name: push changes if they exist
if: env.changes_exist == 'true'
run: |
git push origin HEAD:refs/heads/main
git push origin HEAD:refs/heads/main --tags

- name: create release on new tag if new changes exist
if: env.changes_exist == 'true'
run: |
TAG_NAME=$(git describe --tags $(git rev-list --tags --max-count=1))
echo $TAG_NAME
gh release create "$TAG_NAME" \
--title "$TAG_NAME" \
--notes "See: https://github.com/astral-sh/uv/releases/tag/$TAG_NAME" \
--latest
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
20 changes: 0 additions & 20 deletions .github/workflows/release.yml
View file
Open in desktop

This file was deleted.