(GH-1113) (GH-917) Fix forge-token handling #1121
Merged
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
sanfrancrisko
changed the title
sanfrancrisko
changed the title
forge-token in release cmd
Codecov Report
@@ Coverage Diff @@
## main #1121 +/- ##
==========================================
- Coverage 91.25% 91.24% -0.02%
==========================================
Files 137 137
Lines 5570 5571 +1
==========================================
Hits 5083 5083
- Misses 487 488 +1
Continue to review full report at Codecov.
|
|
added 2 commits
July 7, 2021 18:53
Prior to this fix, the `forge-token` option defined in the PDK::CLI's `release` subcommand did not have the `argument:` attribute set. This meant that the underlying Cri lib treated it as a boolean option and the forge token value was set to `true` and passed to the `PDK::Module::Release` object. This fix sets the `argument:` attribute with the value `:optional`, which allows it to be optional but treated as an arg that needs the value extracted after `=`
Prior to this commit, when running `pdk release` and stepping through the interview questions, even if you selected `N` to the publish forge question, a warning would still display stating: ```sh pdk (ERROR): Missing forge-token option ``` This commit sets the `:skip_publish` option to true in `opts` that gets passed to the `PDK::Module::Release` object on init. This will ensure that the `skip_publish?` call returns `true` and the Forge token check is not performed. This was incorrectly asserted as resolved in `2.1.1` of the PDK.
sanfrancrisko
changed the title
forge-token in release cmdforge-token; Fix incorrect 'Missing forge-token' error throwing
sanfrancrisko
changed the title
forge-token; Fix incorrect 'Missing forge-token' error throwingforge-token; (GH-917)Fix incorrect 'Missing forge-token' error throwing
sanfrancrisko
changed the title
forge-token; (GH-917)Fix incorrect 'Missing forge-token' error throwingforge-token; (GH-917) Fix incorrect 'Missing forge-token' error throwing
jpogran
changed the title
forge-token; (GH-917) Fix incorrect 'Missing forge-token' error throwing
jpogran
approved these changes
Jul 7, 2021
da-ar
approved these changes
Jul 8, 2021
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.
This PR contains two fixes:
(GH-1113) Fix incorrect opt type for
forge-tokenPrior to this fix, the
forge-tokenoption defined in the PDK::CLI'sreleasesubcommand did not have theargument:attribute set.This meant that the underlying Cri lib treated it as a boolean option
and the forge token value was set to
trueand passed to thePDK::Module::Releaseobject.This fix sets the
argument:attribute with the value:required,which emulates the way it is set in the
publishsubcommand.Closes: #1113
(GH-917) Fix incorrect 'Missing forge-token' error throwing
Prior to this fix, the
forge-tokenoption defined in the PDK::CLI'sreleasesubcommand did not have theargument:attribute set.This meant that the underlying Cri lib treated it as a boolean option
and the forge token value was set to
trueand passed to thePDK::Module::Releaseobject.This fix sets the
argument:attribute with the value:optional,which allows it to be optional but treated as an arg that needs the
value extracted after
=Resolves: #917