nedbat / cog

Small bits of Python computation for static files
MIT License
340 stars 26 forks source link

Refactors (including optparse → argparse), and add long options #35

Closed vergenzt closed 4 months ago

vergenzt commented 4 months ago

Hi! I had fun with this one. 🙂 Let me know if you prefer that I break out separate PRs for the individual refactor commits. I kept them in one PR for now since the "business value" for the first two isn't as clear until you get to the point of adding long options (where using snake_case gets you a free implicit dest parameter). Though IMO the majority of the value is in consistency with broader Python ecosystem conventions.

(Which, admittedly, is funny to see alongside the camelCase API for the aging unittest module. 🤦🏻‍♂️ I still think it's worth using snake_case overall outside of those API calls tho, personally.)

If you object to any part of this that's totally fine - I had fun and learned a bunch doing these so the effort wasn't wasted regardless. 🙂 I'm happy to make any changes you prefer. I made a few executive decisions along the way, any of which you, as the maintainer, are of course welcome to disagree with and request changes to.

nedbat commented 4 months ago

Wow, this is a lot. I know how these kind of refactors go, they kind of snowball. It would be easier to review in smaller pieces, but that might not be possible.

vergenzt commented 4 months ago

Yeah sorry for the shock! It's currently split up commit-by-commit, but I can split it into separate PRs for ease of review 👌

vergenzt commented 4 months ago

Closing in favor of separate PRs

vergenzt commented 4 months ago

Followup PRs are going to be

Unfortunately per https://github.com/orgs/community/discussions/4477 there doesn't appear to be a way to create dependent PRs. I'll create "draft" PRs within my fork in case you want to comment on them all at the same time, otherwise I can open PRs one at a time.

Again, if you don't like any of the changes for any reason it will not hurt my feelings if you reject any of them. 🙂 I'm doing this mainly for fun.