Add flake8 to travis tests

Xarthisius commented 5 years ago

@mikedarcy I added flake8 linting to Travis, which should prevent #31 from happening again. This PR also includes another minor fix.

Xarthisius commented 5 years ago

There's also:

./test/test_bdbagit.py:1059:34: F821 undefined name 'unicode'
            self.unicode_class = unicode
                                 ^
1     F821 undefined name 'unicode'
1

which should be addressed, but since it's in the tests it's less important.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 248

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.588%

Totals
Change from base Build 247:	0.0%
Covered Lines:	1449
Relevant Lines:	1565

Pull Request Test Coverage Report for Build 248

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.588%

Totals
Change from base Build 247:	0.0%
Covered Lines:	1449
Relevant Lines:	1565

Pull Request Test Coverage Report for Build 249

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.06%) to 92.524%

Totals
Change from base Build 247:	-0.06%
Covered Lines:	1448
Relevant Lines:	1565

Pull Request Test Coverage Report for Build 248

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.588%

Totals
Change from base Build 247:	0.0%
Covered Lines:	1449
Relevant Lines:	1565

💛 - Coveralls

Xarthisius commented 5 years ago

huh, no flake8 for python 3.3. It reached EoL on 2017-09-29. How about dropping it from Travis test matrix?

mikedarcy commented 5 years ago

There's also:
./test/test_bdbagit.py:1059:34: F821 undefined name 'unicode'
            self.unicode_class = unicode
                                 ^
1     F821 undefined name 'unicode'
1
which should be addressed, but since it's in the tests it's less important.

This is part of a conditionalized PY2/PY3 (and was actually inherited from bagit), so the code is actually OK:

        if sys.version_info >= (3, ):
            self.unicode_class = str
        else:
            self.unicode_class = unicode

I am not that familiar with flake8, but if it can be made to understand the subtlety here (or the code can be rewritten in such a way to not cause it generate a false negative) then I am fine with it.

Xarthisius commented 5 years ago

I am not that familiar with flake8, but if it can be made to understand the subtlety here (or the code can be rewritten in such a way to not cause it generate a false negative) then I am fine with it.

Yup, you can make it shut up by adding # NOQA comment, which I did and I removed the --exclude=test* from flake8 invocation.

mikedarcy commented 5 years ago

huh, no flake8 for python 3.3. It reached EoL on 2017-09-29. How about dropping it from Travis test matrix?

I think that is part of a bigger decision to drop 3.3 (and 3.4) support overall. I am not opposed to it in principle since they are EoL but I am not sure I want to rush into it. That is something that would necessitate a minor version bump (to 1.6.0), which is fine, but also could be timed to include some other improvements.

I like the idea of the flake8 linting but would like some time to look into it myself. In the meantime, I will incorporate the fix for the undefined false in bdbag_ro.py from e745f98 and cut a new micro release 1.5.4 and upload it to PyPi this week.

Xarthisius commented 5 years ago

Sure, I'm fairly certain that before_install with flake8 can be somehow excluded in travis matrix, but I'm not that familiar with its config. I'll look it up and get back with a working setup.

mikedarcy commented 3 years ago

Finally got this included. Thanks for the contrib @Xarthisius!

fair-research / bdbag