fix: auto release on mirror updates#83
fix: auto release on mirror updates#83zanieb merged 2 commits intoastral-sh:mainfrom CoderJoshDK:main
Conversation
|
Let us make sure this makes sense and is valid. Once this is merged and an auto release actually works, I will add this change into https://github.com/astral-sh/uv-pre-commit/ |
| - 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
And then changed main.yml to look like:
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
As mentioned in astral-sh/ruff-pre-commit#82 / astral-sh/ruff-pre-commit#83, the auto release function doesn't work. This fixes that by checking for changes and setting a flag. If the flag is set, it will also create a new release through gh cli. Another note is that the `actions/setup-python` is updated from `v4` → `v5` This is being done now because it has been working well for the ruff-pre-commit repo (I think?)
As mentioned in #82, there are issues with having a workflow triggered by the work done by a different workflow. To keep the functionality of the auto release workflow, the two files are being merged into one.
Closes #82