Skip to content

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