-
Notifications
You must be signed in to change notification settings - Fork 43
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(versions): general refactors to handle flag edge cases #861
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
kanadgupta
added
bug
Something isn't working
refactor
Issues about tackling technical debt
command:versions
Issues pertaining to the `versions` commands
labels
Aug 18, 2023
major bummer that prompts.inject doesn't play nicely with the `type` values in the prompt definitions. figured this out by just playing around locally 😞
kind of weird, but our PUT /version endpoint requires a version as both the path param and as a body param, even if you aren't changing the name of the version. and passing in an empty string for the latter will result in the endpoint shitting the bed. that's API v1 for ya, folks!
kanadgupta
requested review from
erunion,
t-tullis,
a team and
mosesj-readme
and removed request for
a team and
mosesj-readme
August 18, 2023 21:52
erunion
approved these changes
Aug 18, 2023
kanadgupta
pushed a commit
that referenced
this pull request
Aug 18, 2023
## [8.6.5-next.6](v8.6.5-next.5...v8.6.5-next.6) (2023-08-18) ### Bug Fixes * **versions:** general refactors to handle flag edge cases ([#861](#861)) ([e316139](e316139)) [skip ci]
kanadgupta
pushed a commit
that referenced
this pull request
Aug 21, 2023
## [8.6.5](v8.6.4...v8.6.5) (2023-08-21) ### Bug Fixes * **deps:** upgrading out of date deps ([#836](#836)) ([8600554](8600554)) * **npm:** revert shrinkwrap changes ([#824](#824)) ([f96e6dc](f96e6dc)) * **openapi/inspect:** small url formatting error ([#855](#855)) ([c54e289](c54e289)) * **reducer:** quirk with security schemes sometimes getting removed ([#867](#867)) ([be2e037](be2e037)) * **security:** don't publish certain files to npm ([a83fe1c](a83fe1c)), closes [/socket.dev/npm/package/rdme/files/8.6.5-next.2/bin/docker.js#T167-182](https://github.com//socket.dev/npm/package/rdme/files/8.6.5-next.2/bin/docker.js/issues/T167-182) * typo ([#852](#852)) ([26c10e5](26c10e5)) * **versions:** general refactors to handle flag edge cases ([#861](#861)) ([e316139](e316139)) [skip ci]
This was referenced Aug 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
command:versions
Issues pertaining to the `versions` commands
refactor
Issues about tackling technical debt
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.
🧰 Changes
Per RM-7747, our
versions:create
andversions:update
commands don't properly set boolean values correctly, for example:The code for prompting the user and constructing the request payloads is all very messy and hard to follow, so this PR utilizes the incredibly handy
prompts.override
method to vastly simplify things and improve readability in a few places, which in turn fixes a few weird bugs:castStringOptToBool
that properly takes our string command line opts and sets them to their boolean counterparts. Now when a user passes--beta=true
or--beta=false
, we actually pass those into the payload as proper booleans! We also now throw an error if the value for these flags is a string other thantrue
orfalse
.versions:update
command: if a user doesn't want to change the name of the version, they previously had to re-enter the name of the version. Now if they just hit enter, we'll default to the existing version name.codename
option, we no longer send thecodename
field as an empty string in the request payloadversions:create
command was missing thedeprecated
flag? This PR backfills that.🧬 QA & Testing
If tests pass then we should be good, but feel free to try a few flows with the
versions:create
andversions:update
commands to see if everything works as expected.