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.

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

Migrate to Cobra #5

Open
rwx-yxu opened this issue Apr 21, 2024 · 12 comments · May be fixed by #10
Open

Migrate to Cobra #5

rwx-yxu opened this issue Apr 21, 2024 · 12 comments · May be fixed by #10
Assignees

Comments

@rwx-yxu
Copy link
Owner

rwx-yxu commented Apr 21, 2024

Have a Cobra implementation of the weather command just to have experience with a more popular go cli framework compared to bonzai.

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 21, 2024

Weather uses the vars branch to have persistent storage for API Key, location and region ids. Cobra has no equivalent command package. Will have to either set them as env vars or use persistent flags.

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 21, 2024

Breakdown of the current command map:

  • weather
    • site
      • list
      • find
      • set
      • forecast

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 21, 2024

I could use Viper to load in a env file from os.UserCacheDir() I can put this stuff in the app package? or make a new package called conf?
https://dev.to/techschoolguru/load-config-from-file-environment-variables-in-golang-with-viper-2j2d

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 22, 2024

Having mixed thoughts on the project structure. Git hub gh cli groups the cmds by package and each sub command gets its own package. This does mean that you will have multiple create packages in different places for common command names. I did initially think that there would be a conflict but from go pov it would package them together even if the same package names are in different directories during compile. Not entirely sure this is a good approach either.

Option 1:

  • cmd/
    • site/
      • list/
      • find/

Option 2:

  • cmd/
    • site.go
  • site/
  • region/

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 22, 2024

With option 2, the business logic is lifted to a higher level. I also put all site cmds, including its sub commands inside site.go and have the business logic live in site/ and region/.

Whereas with option 1, there is command level isolation. More directory depth.

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 22, 2024

Reviewing what I have now, lets keep it simple and go for option 2. If it gets too large, then I can look to refactor to option 1.

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 23, 2024

Or alternatively, I can still put site.go into site/ and rename to cmd.go and export Cmd. So imports to the root cmd will be site.Cmd

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 26, 2024

Actually, cobra already has a OnInitialize function to load in config file function. This effectively makes the conf package redundant.

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 26, 2024

Should be added to the root node based.

@rwx-yxu
Copy link
Owner Author

rwx-yxu commented Apr 28, 2024

Actual implementation is still to keep the conf sub command because it's useful to have around as a way to output the conf as a command

This was referenced Apr 28, 2024
@rwx-yxu
Copy link
Owner Author

rwx-yxu commented May 7, 2024

Current command tree:

weather conf init
weather conf data

weather site list
weather site set
weather site forecast

@rwx-yxu rwx-yxu linked a pull request Jul 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant