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

fix config file in $HOME dir and other refactors #27

Merged
merged 7 commits into from
Mar 20, 2024

Conversation

sebastianwebber
Copy link
Contributor

In summary, I moved some functions to a config package to extract the main package's config responsibility.

I also added some logger config to help troubleshoot the config file locations, which will work like this:
image

It will use the first file found like this:
image

Feel free to suggest any necessary changes.

It may close #26.

this commit extracts the load config logic to a proper package.
this commit adds a logger to help troubleshot the check for config files.
this commit changes the way of the files are found, following the rules below:

1. If the current directory contains a .meteor.json file, it will be used.
2. If the current directory does not contain a .meteor.json file, the parent
3. IF parent doesn't contain the .meteor.json file, the search will continue until the home directory is reached.
4. If no .meteor.json file is found, look in ~/.config/meteor/config.json
5. If no .meteor.json file is found, return an error
Signed-off-by: Sebastian Webber <[email protected]>
@sebastianwebber
Copy link
Contributor Author

also fixed the issue that passed the flag args to the git command:

❯ go run main.go git.go config.go -L debug -- -s -S
2024-03-20 03:07:57 DEBU checking for config file path=/Users/seba/projetos/github.com/sebastianwebber/meteor/.meteor.json
2024-03-20 03:07:57 DEBU checking for config file path=/Users/seba/projetos/github.com/sebastianwebber/.meteor.json
2024-03-20 03:07:57 DEBU found config file path=/Users/seba/projetos/github.com/sebastianwebber/.meteor.json
2024-03-20 03:07:57 DEBU loading config file path=/Users/seba/projetos/github.com/sebastianwebber/.meteor.json
[fix-config-file d6dc116] fix: ignore non-parsed args
 1 file changed, 1 insertion(+), 1 deletion(-)

@sebastianwebber
Copy link
Contributor Author

ideally we could use cobra to handle the command line args, let me know if you would like a PR to handle this.

this commit renames Prefixes.Option to .Options to keep
the naming convention defined in other objects.

Signed-off-by: Sebastian Webber <[email protected]>
@stefanlogue
Copy link
Owner

This looks good, I'll just need to get some time (hopefully this evening) to pull it down and give it a quick test. It may close #26, but this isn't the fix I had in mind for that issue. Obviously, if this does fix it then even better!

In regards to using Cobra to handle the command line arguments, I'd like to avoid adding unnecessary complexity to the tool as much as possible for now. If you can give me a good enough reason to make the move at this stage, then I'm all ears! But I feel like it's something I'd rather do when the complexity it adds is more justified

main.go Outdated
flag.BoolP("version", "v", false, "show version")
flag.StringVarP(&logLevel, "log-level", "L", "info", "Log level (debug, info, warn, error, fatal, panic)")
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need all of these different debug levels? I think having a flag for debugging is a great idea (I wish I had this when I was originally writing the code for loading the config files!!), I think we maybe only need to be able to toggle debugging on or off, which could be achieved like meteor --debug or meteor -D, as git commit does not use the --debug or -D flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me update that then. hold on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some changes:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

❯ go run main.go git.go config.go --help
Usage of /var/folders/yx/7rrwmnvd3d14_qc362y5yt300000gn/T/go-build713247778/b001/exe/main:
  -D, --debug     enable debug mode
  -v, --version   show version
pflag: help requested
exit status 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About cobra, let's discuss this in a new PR or issue.

Copy link
Owner

Choose a reason for hiding this comment

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

Added some changes: image

This looks good, but I'm wondering if we want to raise a warning when a user has no config file? I imagine the vast majority of users won't need a config file at all if they just want to do vanilla conventional commits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, let's move to debug then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

807b625 should fix the problem.

main.go Show resolved Hide resolved
When the config file is not found the error will be suppresed but
it will be displayed in the debug messages with the `--debug` extra
arg.

Signed-off-by: Sebastian Webber <[email protected]>
Copy link
Owner

@stefanlogue stefanlogue left a comment

Choose a reason for hiding this comment

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

LGTM!

@stefanlogue stefanlogue merged commit 52cf57b into stefanlogue:main Mar 20, 2024
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 this pull request may close these issues.

Global meteor.json
2 participants