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

Fix updating draft releases #255

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

vadz
Copy link

@vadz vadz commented Sep 4, 2022

Sorry again, after opening #254 and then closing it because I've noticed the already existing #245, I'm finally opening this one because, after looking at #245, I believe this one is slightly better because it correctly uses createRelease() if a draft release does not exist, rather than using updateRelease() even in this case. Even if the latter works too, using createRelease() looks preferable and, at the very least, results in the expected message in the logs.

I've also changed the way we're looking for the existing draft release in this one because using the (empty) tag doesn't seem to be the right thing to do neither.

Please note that although it looks like there are many changes here, using git diff -w --color-moved --color-words shows that most of them are actually due to just moving the code around.

No real changes yet, this is just a small cleanup before the upcoming
commits.
The tag is typically (or maybe even always) empty for the draft
releases and can't be really used, while the release name will normally
be unique, so use it as the key.
Don't just return the release without doing anything else, notably this
didn't allow updating an existing draft release body.

This change required refactoring the code to use a new helper
find_existing_release() function, but the only effective change is that
the main release() function doesn't return early any more if a draft
release already exists.

This commit is best viewed ignoring whitespace-only changes and using
git --color-moved option.
@vadz
Copy link
Author

vadz commented Feb 9, 2023

I've rebased the original branch on the latest master to avoid conflicts due to formatting changes.

Please let me know if you have any questions or comments about this PR, it would be really nice to see it merged.

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.

None yet

1 participant