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

Align black config with Haystack repo #32

Merged
merged 4 commits into from
Oct 10, 2022

Conversation

anakin87
Copy link
Member

In #4, I said:

If I try to run pre-commit run --all-files, it seems that black-jupyter (22.6.0) wants to reformat all the notebooks.

@masci:

we never run a formatter on the notebooks, I think this is a good opportunity to do it. The only problem I see, it's already hard to port changes from the haystack repo now, if we re-format the files it'll be impossible. So I would re-format all the things only if we're close enough to remove the files from the haystack repo. WDYT?

@masci @TuanaCelik I found out what the problem is:
in Haystack repo we are running black with the configurations specified in pyproject.toml:

[tool.black]
line-length = 120
skip_magic_trailing_comma = true  # For compatibility with pydoc>=4.6, check if still needed.

Proposed changes

In this PR, I fix this misalignment, by adding these parameters to the black entry in .pre-commit-config.yaml
Another option could be to add pyproject.toml to this repo: maybe it is premature or not wanted/needed.

@anakin87 anakin87 requested a review from masci September 23, 2022 20:53
Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

I'd add a pyproject file to configure black, so we can reuse it in the CI

@anakin87
Copy link
Member Author

anakin87 commented Oct 4, 2022

@masci I added a minimal pyproject file, based on that of the main repo.
Feel free to review/edit it.

@anakin87 anakin87 requested a review from masci October 4, 2022 19:28
pyproject.toml Show resolved Hide resolved
@anakin87 anakin87 requested a review from masci October 10, 2022 11:00
Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

Thanks!

@masci masci merged commit 013ca51 into deepset-ai:main Oct 10, 2022
@bilgeyucel bilgeyucel mentioned this pull request Nov 24, 2022
2 tasks
@anakin87 anakin87 deleted the fix_black_config branch December 12, 2022 11:08
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

2 participants