Fix updating draft releases #255
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
force-pushed
the
fix-draft-update
branch
from
1460c8b to
4c1f6af
Compare
|
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. |
|
@vadz can you fix the merge conflicts? Thanks! |
|
There are too many conflicts with fb2d031 (Fix missing update release body (#365), 2024-07-17) and I think that PR fixed the same problem, albeit in a somewhat different way. I haven't tested if it's really fixed for me yet however, but if there are no known problems with the other PR, this one can be closed. |
|
@vadz sounds great, feel free to throw another PR. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 usingupdateRelease()even in this case. Even if the latter works too, usingcreateRelease()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-wordsshows that most of them are actually due to just moving the code around.