Skip to content

Use Typer instead of argparse #49

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

Merged
merged 27 commits into from
Feb 9, 2023

Conversation

paketb0te
Copy link
Contributor

The PR for #43 :)

@initialcommit-io
Copy link
Contributor

@paketb0te Thanks for this! Just 2 comments for now:

  • Sorry to ask this since we discussed changing the file names, but I think that might interfere with Git blame by replacing the whole file and its content. Can you leave the file names as they were (it's fine to keep the Class names updated to the new values), and I'll rename them in a future commit?

  • It looks like self.maxrefs is still there in some places. Do you want to remove that?

@paketb0te
Copy link
Contributor Author

paketb0te commented Feb 9, 2023

@initialcommit-io of course, I just realized that I still have both files for each command in my branch (kept the original ones to look at while modifying the new ones).

I moved everything back into the original files now (so that git blame works correctly).

Since we introduced a new dependency on typer, we should probably start including something like a requirements.txt (or does setup.py automatically install dependencies from PyPi? 🤔)

Oh and I removed the obsolete maxrefs :)

@initialcommit-io
Copy link
Contributor

@paketb0te Thanks! Setup.py does automatically include the dependencies, so we should be ok for now, but we do still have the other open issue for the dependency system upgrade...

@initialcommit-io initialcommit-io merged commit cbd990d into initialcommit-com:main Feb 9, 2023
@paketb0te paketb0te deleted the typer branch February 10, 2023 08:42
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.

2 participants