search

aau-network-security / richkit

Domain Enrichment Toolkit $ pip install richkit
https://pypi.org/project/richkit/
MIT License
11 stars 3 forks source link

lint project before commit prevents fails in CI #118

Closed mrtrkmn closed 4 years ago

mrtrkmn commented 4 years ago

fixes #117

mrtrkmn commented 4 years ago

Note that the failing test case is NOT about the changes in this pull request.

mrtrkmn commented 4 years ago

👍 overall, but one point I'd like to have clarified; This seems to (potentially) modify the working tree with autopep8, but after committer decides what is to be commit and after creating the commit message. Do we understand where the changes by autopep8 will go? It seems to me that, as long as autopep8 is available, the staged changes will be committed whether there are pep8 corrections to be made or not, and any changes by autopep8 will be left in the working tree. Is that correctly understood and is that as intended?

Yes good point to mention, it is annoying to review changes which are caused by autopep8 in one commit, however it cannot be seen multiple times. Since other parts of the project will be already linted and just the part where changes made will be effected by this hook, . So, if the code base already linted by autopep8, the parts other than the actual changes will not be affected.

I hope, I have understood your concerns correctly and responded accordingly.

kidmose commented 4 years ago

I hope, I have understood your concerns correctly and responded accordingly.

I'm not sure, but it's minor so let's let it fly :)

mrtrkmn commented 4 years ago

I'm not sure, but it's minor so let's let it fly :)

I mean, once everything is linted, there will be no more linting other than changed files for a commit.

mrtrkmn commented 4 years ago

I will merge at the end, this way develop will be linted and include latest changes.