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

Pre commit test #93

Merged
merged 26 commits into from
Sep 29, 2023
Merged

Pre commit test #93

merged 26 commits into from
Sep 29, 2023

Conversation

pavelvazquez
Copy link
Contributor

Tested the pre-commit requirement for developers but seems to be very slow

@sveinugu sveinugu self-requested a review May 15, 2023 05:00
Copy link
Contributor

@sveinugu sveinugu 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 very late review. Have had to prioritise stuff related to biohackathon/conferences.

As I understand it, you have tried to make poetry work well with pre-commit and your solution works, but I found that it was not recommended by the developer of pre-commit (https://stackoverflow.com/a/72888197). Due to the complexity of this, I went ahead and reconfigured pre-commit to handle the dependencies for formatting and linting alone (removing these dependencies from poetry). PyCharm File Watch config needs to be changed accordingly (info in CONTRIBUTING.md). Documentation on installation, use, and development setup has been added.

However, I discovered that the Flake8 config has been incorrect for a long time, perhaps always, which have caused most errors to be ignored. I started fixing some of the errors that now appeared, but then understood that fixing the rest is good training for you. So fix the rest of the flake8 errors before I will merge the PR.

GitHub Actions workflows have been updated.

@sveinugu sveinugu merged commit 4241123 into main Sep 29, 2023
5 checks passed
@sveinugu sveinugu deleted the pre-commit-test branch September 29, 2023 13:20
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.

3 participants