← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~alisonken1/openlp/pjlink2-f into lp:openlp

 

Review: Needs Fixing

In general it looks good, but there are 2 things I'm not too happy about, the use of "# noqa" and the "possible future imports".
While I think using pep8/pycodestyle, flake8 and pylint is great, I'm not too happy about us having to "pollute" the source code with hints of where to ignore stuff. IMHO this should be done in the setup.cfg.
I do not think "possible future imports" belongs in trunk. While I do understand that you are doing a series of merge requests and that you plan to update the imports later on, I think we should strive to keep trunk as "clean" as possible.
If my fellow code reviewers does not agree with my points above, I am willing to talk about it...
-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326265
Your team OpenLP Core is subscribed to branch lp:openlp.


References