-
Notifications
You must be signed in to change notification settings - Fork 5
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
Better settings parsing and pip installation handling #106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And of course some nagging about the commit messages. I appreciate that they now contain more information. But, reading them in the console is not that great:
That is why one should break the lines, as explained here. A common guide line is 50 characters for the subject and 72 for the body. Pro tip: Editors can help you with that.
So, your commit message I would change to something like this:
Check multiple places for settings file
Also, rename `settings.yaml` to `goosebit.yaml` to make it
distinguishable from other files.
The user can now pass `GOOSEBIT_SETTINGS` to customize settings
location, as well as gooseBit auto checking `/etc/goosebit.yaml`, the
root of the project directory, and the current working directory when it
is run. This should allow for more optionality when loading settings
files.
Fixes: #103
I wonder if this is a good case for pre-commit? |
|
93c2429
to
a1f4cc2
Compare
|
I'll want to test this... |
adb0de3
to
4b3e3b5
Compare
4b3e3b5
to
78d48a8
Compare
c980c16
to
516b311
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I would squash the last commit. No need for intermediate (broken) versions.
Will do, just give me a bit. |
8639a81
to
7064c53
Compare
…settings. Use can now pass `GOOSEBIT_SETTINGS` to customize settings location, as well as goosebit auto checking `/etc/goosebit` if it exists, the root of the project directory, and the current working directory when it is run. This should allow for more optionality when loading settings files. Renaming settings to `goosebit.yaml` is mainly to specify what the file is when running from CWD, it should be distinct from other files. Fixes: #103
7064c53
to
9106c3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great improvement, thanks!
Hopefully a better solution to settings parsing than #105, uses
platformdirs
to find device specific directories where possible. Will also log a warning about missing settings file, but will still check in project root for development.Also moved
main.py
to__main__.py
for better pip installation support, so it can be run withpython -m goosebit
.Fixes: #103