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

Fix black #1688

Merged
merged 5 commits into from Jan 25, 2022
Merged

Fix black #1688

merged 5 commits into from Jan 25, 2022

Conversation

eggplants
Copy link
Contributor

@eggplants eggplants commented Jan 24, 2022

Ref: #1687

Proposed Changes

  • fix black command argument in CI (./rdflib -> .)
  • format with black --config black.toml .
    • formatted:
      • rdflib/namespace/_GEO.py
      • rdflib/plugins/parsers/hext.py
      • rdflib/plugins/serializers/hext.py
  • sort dev dependencies in requirements.dev.txt

@ashleysommer
Copy link
Contributor

fix black command argument in CI (./rdflib -> .)

It was intentional that we only check for black formatting in the /rdflib subdirectory.

@nicholascar
Do you know why the _GEO.py, hext-parser, and hext-serializer were not Blacked before they were merged?

@nicholascar
Copy link
Member

Do you know why the _GEO.py, hext-parser, and hext-serializer were not Blacked before they were merged?

My mistake? They should have been! Clearly I'm responsible for those particular files...

@aucampia
Copy link
Member

Do you know why the _GEO.py, hext-parser, and hext-serializer were not Blacked before they were merged?

I think with nothing doing any check as it stands it is easy to forget, this may help:

@aucampia
Copy link
Member

fix black command argument in CI (./rdflib -> .)

It was intentional that we only check for black formatting in the /rdflib subdirectory.

The current config should exclude everything else and also relying on config simplifies integration with other tooling. But should be same behavior.

Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Copy link
Member

@aucampia aucampia 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, thanks.

@aucampia
Copy link
Member

actually, black.toml has no effect: https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-via-a-file - so actually for the time being it is quite critical to keep the specific directory specification. I will make a separate pr to rename it to pyproject.toml.

@aucampia
Copy link
Member

Fix for black config: #1692

@nicholascar nicholascar merged commit 48e9d29 into RDFLib:master Jan 25, 2022
@eggplants eggplants deleted the fix_black branch January 25, 2022 03:43
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.

None yet

4 participants