X Tutup
Skip to content

New confirmations#1492

Merged
vilmibm merged 9 commits intocli:trunkfrom
ShubhankarKG:newConfirmations
Aug 25, 2020
Merged

New confirmations#1492
vilmibm merged 9 commits intocli:trunkfrom
ShubhankarKG:newConfirmations

Conversation

@ShubhankarKG
Copy link
Copy Markdown
Contributor

@ShubhankarKG ShubhankarKG commented Aug 6, 2020

Fixes #1396
Fixes #1270
Fixes #798
Ref #1330
Ref #1360

Changes:-

  1. Introduced three new flags for visibility filters :- --public, --private, --internal which need to be passed. Failure to do so invokes an interactive session.

  2. Introduces new flag -y that passes true for all Confirm statements. repo create has two confirm statements :- Confirm submit? and Create local directory. Both shall be true when passed.

3.gh repo create on passing no arguments shall open up the interactive session as noted.

…h meeds to be passed.

2. Interactive repo create when no parameters are passed, i.e `repo create`.
3. Fix tests.

TODO : write a repo in the form of user/repository format when no user is supplied.
@ShubhankarKG
Copy link
Copy Markdown
Contributor Author

Requesting to have a look at this 🙏

Copy link
Copy Markdown

@ampinsk ampinsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks so much for this and apologies for the delay. I reviewed quickly from an experience perspective and had a couple of comments. I'll leave a more thorough review to @mislav or @vilmibm. thanks again!

  1. In this prompt, can the options be in sentence case instead of capitalized?
? Visibility  [Use arrows to move, type to filter]
> PUBLIC
  PRIVATE
  INTERNAL

Should be

? Visibility  [Use arrows to move, type to filter]
> Public
  Private
  Internal

  1. In the final confirmation, the default choice should be yes, not no, and the repository name should have single quotes around it.

This will create name in your current directory. Continue? (y/N) should be This will create 'name' in your current directory. Continue? (Y/n)

@StefanXhunga

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good, thank you! Just one minor change, please.

Name: "repoVisibility",
Prompt: &survey.Select{
Message: "Visibility",
Options: []string{"PUBLIC", "PRIVATE", "INTERNAL"},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should also become Public, Private, Internal

The problem was that opts.confirmSubmit was mutated before reaching doSetup. This commit creates a copy of the initial confirmSubmit value. So the doSetup receives the initial data passed from the command, not the mutated one.
@StefanXhunga

This comment has been minimized.

@vilmibm
Copy link
Copy Markdown
Contributor

vilmibm commented Aug 25, 2020

Thank you for your work and patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants

X Tutup