Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Refactor cherami-admin and cherami-cli code #247

Merged
merged 3 commits into from
Jul 13, 2017
Merged

Refactor cherami-admin and cherami-cli code #247

merged 3 commits into from
Jul 13, 2017

Conversation

kobeyang
Copy link
Contributor

@kobeyang kobeyang commented Jul 7, 2017

Basic idea:

  1. Create folder common under cmd/tools, and create lib.go in it to store the common flags and commands used by both cli and admin.
  2. Delete tools/cli/lib.go and move the corresponding functions to tools/common/lib.go. Add a parameter serviceName to these common functions to differentiate cli and admin.
  3. Cherami-cli will just use the common functions and Cherami-admin will use some admin-specific functions in tools/admin/lib.go.

Not sure whether this is the best way to do this, please leave any suggestion whichever you want.

@CLAassistant
Copy link

CLAassistant commented Jul 7, 2017

CLA assistant check
All committers have signed the CLA.

@kobeyang kobeyang requested review from datoug, kirg and thuningxu July 7, 2017 07:27
@coveralls
Copy link

coveralls commented Jul 7, 2017

Coverage Status

Coverage decreased (-0.02%) to 68.292% when pulling a8246dd on refactor into 51b6724 on master.

@kirg
Copy link
Contributor

kirg commented Jul 7, 2017

@kobeyang i just committed PR #246 that will probably cause some merge conflicts for you .. (sorry about that .. :-))

@kobeyang
Copy link
Contributor Author

Merge with Kiran's change in PR#246.

Copy link
Contributor

@thuningxu thuningxu left a comment

Choose a reason for hiding this comment

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

Please move the menu in cmd/tools/admin/main.go to the library.

Copy link
Contributor

@thuningxu thuningxu left a comment

Choose a reason for hiding this comment

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

Reviewed in person. Generally looks good. Please remove the *Helper functions.

@kobeyang kobeyang merged commit 9a61305 into master Jul 13, 2017
@kobeyang kobeyang deleted the refactor branch July 13, 2017 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants