-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Rewrite publish logic #132
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
9cda15e
Initial attempt to rewrite publish logic
confused-Techie 5911d3f
Condense rename logic during actual publication
confused-Techie 1fd6a5b
Remove duplicate tag declaration
confused-Techie ab1fcd5
Use existing code, ensure to `await`, log for assistance
confused-Techie df51c83
Remove extra logging, fix test to new value
confused-Techie af47f04
Also check for tag existence before erroring out
confused-Techie 1d4852d
Only create and assign tag if there isn't a user provided one
confused-Techie dee2d17
Ensure to only exit early if also missing rename, fix logic error in …
confused-Techie b06f1a7
Fix typo
confused-Techie cf9f1ae
Add some verbose logging
confused-Techie d3153e1
Make verbose option a boolean
confused-Techie a16919e
Make sane length checks, ensure `tag` always has a length
confused-Techie d8ef281
Also protect `version` from being `undefined`
confused-Techie 9baec4c
Finally normalize `rename` to a string
confused-Techie 01af708
Clean up length logic now that we have normalized values
confused-Techie 567b247
Remove outdated verbose flag
confused-Techie 1b9ebd2
Confidently check length now that we normalize variable values
confused-Techie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just commentary, no action needed:
Kind of odd to see that this check was coded wrong from before this PR, I believe it was inadvertently changed as part of the utterly massive promisification PRs, from looking at the blame history.
This change seems legit. All other calls to
logFailure()
are whencode !== 0
, as in error code rather than clean exit code. So yeah, seems legit IMO.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was surprised to see this one broken for so long tbh