-
Notifications
You must be signed in to change notification settings - Fork 27
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
Source code modernization #32
Comments
I have no objections to those changes, I just don't feel much need to make them. I don't use pre-commit hooks, but if we add the ruff checks to CI also, then we can stay in-synch. For the formatting changes, we should add a What substantive changes were you thinking of making? I'd hate for you to put in this work and then for us to disagree on your eventual goal. |
Didn't have much specifics in mind. Mostly was just bored and love this tool and wanted to unblock some previous requests you've mentioned. E.g. 5 years ago you mentioned the coding style is "really archaic". 😆 I think some things I would love to do include:
I'll harbor no expectations that you'll approve a future changes just because you approve an earlier one. 🙂 |
This is totally awesome by the way. I had no idea this is a thing but I love it. |
Hi @nedbat! I've been thinking about a few suggestions / requests for this app, but you've mentioned in a few places that the code style is pretty archaic. I'm interested in helping to modernize it. I'm gonna put my tentative todo list here -- obviously 👍'ing or 👎'ing each PR will be up to you, but if you're up for weighing in here that'd be helpful.
Auto-format Python code using
ruff format
(preferably viapre-commit
hook?)Note: I may go ahead and submit a PR for this one since it's trivial to do.Lint Python code using
ruff check
(preferably viapre-commit
hook?) & apply suggestionsSome of them are more trivial, but I tend to think it's worth it for the consistency. 🤷
Here's the current list I'm seeing:
23 "errors"
Convert
CogOptions
fromgetopt
toargparse
cogapp/options.py
file. And maybe make the usage string autogenerated based on the configured argument help strings?TBD...
The text was updated successfully, but these errors were encountered: