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

pyproject.toml support #893

Merged
merged 10 commits into from
Mar 12, 2021
Merged

Conversation

hirosassa
Copy link
Contributor

@hirosassa hirosassa commented Jan 27, 2021

This PR is for #708

I added pyproject.toml support.
Section syntax of pyproject.toml file is the same as @samypr100 's explanation in #708.

Please let me know if I miss anything.
Thank you.

@google-cla google-cla bot added the cla: yes label Jan 27, 2021
@@ -17,6 +17,8 @@
import re
import textwrap

import toml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[help wanted] I added external dependency because there's no toml loader in standard libraries. Is this OK?

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this to be seamless. So if someone doesn't have the toml package installed, then we just skip this support all together. Is it possible to import the toml conditionally and if it fails to import set a flag or something?

Copy link
Contributor Author

@hirosassa hirosassa Feb 16, 2021

Choose a reason for hiding this comment

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

I agree with you! Fixed this in 58b9034 and 04b1111

@coveralls
Copy link

coveralls commented Feb 7, 2021

Coverage Status

Coverage decreased (-0.8%) to 93.728% when pulling 04b1111 on hirosassa:add-pyproject-integration into a53e340 on google:main.

@hirosassa
Copy link
Contributor Author

@gwelymernans Hi! This PR is ready for review.
If miss anything, please let me know.
Thank you!

Copy link
Member

@bwendling bwendling left a comment

Choose a reason for hiding this comment

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

Sorry for the late review.

@@ -17,6 +17,8 @@
import re
import textwrap

import toml
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this to be seamless. So if someone doesn't have the toml package installed, then we just skip this support all together. Is it possible to import the toml conditionally and if it fails to import set a flag or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants