Skip to content
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

Refactor into submodules #90

Merged
merged 13 commits into from
Jan 5, 2024
Merged

Refactor into submodules #90

merged 13 commits into from
Jan 5, 2024

Conversation

meshy
Copy link
Collaborator

@meshy meshy commented Jan 3, 2024

This is mostly just moving code around, but does add a "SUCCESS" return-code, and do a couple of renames.

I intend to add more commands (as a re-work of #18), and getting this out of the way will make that simpler.

Before this the `mypy_json_report` module was a single file.

By using a `mypy_json_report` package we'll be able to split logic up
into multiple files, and maintain the current namespace.
mypy_json_report/__init__.py Outdated Show resolved Hide resolved
@meshy meshy marked this pull request as ready for review January 3, 2024 16:45
@meshy meshy requested a review from Tenzer January 3, 2024 16:59
Copy link
Member

@Tenzer Tenzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

You should change the entrypoint for the package so it points at the new location for the main function:

mypy-json-report = "mypy_json_report:main"

mypy_json_report/return_codes.py Outdated Show resolved Hide resolved
mypy_json_report/return_codes.py Outdated Show resolved Hide resolved
mypy_json_report/__init__.py Outdated Show resolved Hide resolved
meshy added 12 commits January 4, 2024 09:57
This is consistent with the name of the class.
This will make working with these types easier, and better allows for
future expansion.
Previously, we composed our parsing objects in the same function
as we executed the parsing logic.

This change separates those concerns, and will make splitting up this
file easier.
These tests were only concerned with the parse command, so this name is
more fitting.
While these are not (currently) explicitly used, it's nice to have them
here for reference.
@meshy meshy force-pushed the introduce-modules branch from 3ec3a5c to 0ef89af Compare January 4, 2024 10:52
@meshy
Copy link
Collaborator Author

meshy commented Jan 4, 2024

Thanks @Tenzer. I think I've addressed your points now.

Copy link
Member

@Tenzer Tenzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Tenzer Tenzer merged commit 8247df0 into main Jan 5, 2024
6 checks passed
@Tenzer Tenzer deleted the introduce-modules branch January 5, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants