Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Create convention for organization of CLI helper functions #43

Closed
ianpittwood opened this issue Feb 13, 2020 · 7 comments
Closed

Create convention for organization of CLI helper functions #43

ianpittwood opened this issue Feb 13, 2020 · 7 comments
Assignees
@sirajyasin
Labels
cli Relates to configuration or code for the CLI enhancement New feature or request good first issue Good for newcomers priority/low Items that are considered non-critical for functionality, such as quality of life improvements ready for review Change related to the issue is ready for review refactor Relates to code refactoring
Milestone
betav1

Comments

@ianpittwood
Copy link
Contributor

Relates-To: #27

Problem description (if applicable)
As part of #27, CLI helper functions were moved from the cmd/config package to the pkg/config package. In the associated Gerrit change for #27, @dukov pointed out that we should create a convention for where CLI helper functions are stored inside their respective packages. Creating and implementing this convention will help us better distinguish library code and CLI helper code.

Proposed change
Create and implement a convention for organization of CLI helper functions.
@ianpittwood ianpittwood added enhancement New feature or request question Further information is requested cli Relates to configuration or code for the CLI labels Feb 13, 2020
@jezogwza jezogwza added the priority/low Items that are considered non-critical for functionality, such as quality of life improvements label Feb 19, 2020
@jezogwza jezogwza added this to the betav1 milestone Feb 19, 2020
@jezogwza jezogwza added the good first issue Good for newcomers label Feb 19, 2020
@ianpittwood ianpittwood added refactor Relates to code refactoring and removed question Further information is requested labels Feb 24, 2020
@ianpittwood ianpittwood self-assigned this Feb 24, 2020
@ianpittwood ianpittwood removed their assignment Mar 17, 2020
@sreejithpunnapuzha
Copy link
Member

@sirajyasin is working on this issue. i am not able to assign it to him. can some one help me in assigning this to him?

@ian-howell
Copy link
Contributor

@sreejithpunnapuzha I'm unable to add him as well. I've done some digging into why this keeps happening, and it seems as though Github disallows assigning issues to arbitrary users to prevent abuse. In order to assign a user, that user must either have write permissions to the repo (as of now, I believe this is limited to WC and cores), or they must have left a comment on the issue.

@sirajyasin if you would like me to assign you to this issue, please leave a comment here.

@sirajyasin
Copy link
Contributor

@ian-howell , thanks for looking into the GitHub issue.
Yes, please assign it to me.

@airshipbot airshipbot added the ready for review Change related to the issue is ready for review label Mar 23, 2020
@airshipbot
Copy link

airshipbot commented Mar 23, 2020 •
edited
Loading

Related Change #713755

Subject: Organize CLI helper functions
Link: https://review.opendev.org/713755
Status: MERGED
Owner: Sirajudeen (sirajudeen.yasin@gmail.com)

Approvals

Code-Review
+2 Alexander Hughes
+2 Ian Howell
Verified
+2 Zuul
Workflow
+1 Ian Howell

Last Updated: 2020-04-22 16:45:08 CDT

@sirajyasin
Copy link
Contributor

Removed the cmds.go within config/ and added a helper for config ( config_helper.go ). If this approach is agreed, then we can create helpers for respective packages when required and have the additional validations or any kind of helper functions done there

PS for review in Gerrit

@jezogwza jezogwza mentioned this issue Apr 8, 2020
Closed
airshipbot pushed a commit that referenced this issue Apr 22, 2020
@sirajyasin
Organize CLI helper functions
01d66c8 
* moving cli helper functions to the respective packages with _helper.
* removed cmds.go within pkg/ dir and added config_helper.go

* In the first commit(https://review.opendev.org/#/c/707216/) commit CLI helper
  functions were moved from the cmd/config package to the pkg/config package
* Now we are trying to move the CLI helper functions within their respective packages instead of a command cmds.go.
* Creating and implementing this convention will help us better distinguish library code and CLI helper code
* Added MaxNArgs as 1 for get-credentials and updated test cases

Relates-To: #43
Change-Id: Ibb8139102bd98c6c7596bd97377cf83381cbeb00
@sirajyasin
Copy link
Contributor

The commit is merged. So this issue can be marked closed. Going further i will follow the comment convention to auto close.

@airshipbot
Copy link

A [Related Change](https://review.opendev.org/778359 was merged. This issue may be ready to close.

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

@sirajyasin sirajyasin

Labels
cli Relates to configuration or code for the CLI enhancement New feature or request good first issue Good for newcomers priority/low Items that are considered non-critical for functionality, such as quality of life improvements ready for review Change related to the issue is ready for review refactor Relates to code refactoring
Projects
None yet
Milestone
betav1
Development

No branches or pull requests
6 participants
@jezogwza @ianpittwood @ian-howell @sirajyasin @sreejithpunnapuzha @airshipbot