-
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
feat(breaking/version): raw versions only, usage cleanup #603
Conversation
@@ -6,6 +6,10 @@ import configStore from '../../src/lib/configstore'; | |||
const cmd = new Command(); | |||
|
|||
describe('rdme logout', () => { | |||
afterEach(() => { | |||
configStore.clear(); |
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.
Oh is us not clearing this out why sometimes the login test flakes out?
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.
I think so 😬 the test bed seems a little flakier now that I added so many tests messing with configstore
so I'm hoping this addresses that
@@ -152,17 +152,19 @@ export default class Command { | |||
{ | |||
name: 'main', | |||
type: String, | |||
description: 'Should this version be the primary (default) version for your project?', | |||
description: | |||
"Should this version be the primary (default) version for your project? (Must be 'true' or 'false')", |
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.
Adding so much boilerplate into the help screens for "must be true or false" seems like a lot.
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.
yeah I don't love this but not sure if it's clear otherwise since the user is just supposed to pass in a string. ideally this should be a boolean but I've given these commands more love than they already deserve lol
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.
really not sure why i ever added that table view in but glad to see it hit da bricks
🧰 Changes
versions
command and removes all logic/deps pertaining to table generationversions:update
command now accepts theversion
parameter as the default option (rather than via a--version
flag) so the usage is consistent withversions:create
andversions:delete
versions
commandsconfigstore
for tests that interact with itVersion
type (i.e. the version object that isPOST
-ed and received from our API) and uses it throughout the codebase where applicable.🧬 QA & Testing
Do the docs changes look good? And do tests pass?