jayvdb created this task. jayvdb added subscribers: jayvdb, valhallasw, XZise, Xqt. jayvdb added a project: pywikibot-core. Restricted Application added subscribers: Aklapper, pywikipedia-bugs.
TASK DESCRIPTION There is a new pep8 rule which was added in July 2014, and will be released soon. If it is deployed on the build machines, it would prevent any other changes from being merged.
The rule is
E402 module level import not at top of file, and looks like
pywikibot/data/api.py:10:1: E402 module level import not at top of file
https://github.com/jcrocholl/pep8/issues/264 https://github.com/jcrocholl/pep8/commit/1ee296bca0fa611d3dbe87c5c5c8009e448...
pywikibot has several imports not at the top of the file, due to cyclic dependencies, but that number is small compared to the number of these errors caused by __version__ appearing before the imports.
Note that pep8 also needed to change their code to relocate __version__ to appear below imports.
https://github.com/jcrocholl/pep8/commit/373e0ac1138f0e24422b5e2e78f02ed0557...
There is a broader issue of the usefulness of these __version__ variables for every file, and there is some $Id$ voodoo in bot.py:740-745 which we should revisit.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise added a comment.
Of what usefulness is the version in all files anyway? As far as I could see it isn't the last commit hash that file was changed.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise added a comment.
This could also be handled together with https://phabricator.wikimedia.org/T73788 depending on what we plan to do with `__version__`.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise edited the task description. XZise set Security to none.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise added a comment.
And another question: Does this affect “conditional imports”, for example when we import depending on whether we need it for Python 2 or Python 3?
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a comment.
In https://phabricator.wikimedia.org/T87409#990824, @XZise wrote:
And another question: Does this affect “conditional imports”, for example when we import depending on whether we need it for Python 2 or Python 3?
The upcoming version does have dedicated code to allow conditional imports, but we'd need to verify that it allows all of our oddities and file issues if it doesnt when we feel that it should.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a comment.
In https://phabricator.wikimedia.org/T87409#990814, @XZise wrote:
Of what usefulness is the version in all files anyway? As far as I could see it isn't the last commit hash that file was changed.
It is the object hash. It could be useful to determine if the file is modified, both locally or via a reported log, but it isnt necessary to achieve that goal either. It may not even be the most efficient way to do it.
$ git hash-object pywikibot/version.py ce957752e3b8bfb4ecca30c89785b80b93c59c8c $ grep __version__ pywikibot/version.py __version__ = '$Id: ce957752e3b8bfb4ecca30c89785b80b93c59c8c $' $ echo '# foo ' >> pywikibot/version.py $ git hash-object pywikibot/version.py bf31be3d89f9f93728390f8e0a37873ad0fa18b6
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise added a comment.
Okay but I doubt the usefulness. When someone has a problem and gives the version number how does it help? Can I search for a file with the given hash in the git file history? Otherwise I don't see how that hash helps… even if you only want to detect if the file is changed you need to know the original hash and for that you need to know the version the file is in.
I'd say it should be removed any and only used where we actually have a version number like in `__init__.__version__` to tell the current package version (if that is not already done automatically). On that topic it would be probably helpful if the nightlies have something like '2.0b3-20150124' so we know when that nightly was build (and if possible maybe the short git hash just to be sure). But again not in every file but only in the `__init__.py` probably.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a subscriber: Ladsgroup. jayvdb added a comment.
@ladsgroup knows about the nightly system.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a comment.
IMO it is definitely useless for pywikibot, but it might be useful for versioning of scripts , or reporting script modification status via user-agent ( https://phabricator.wikimedia.org/T57016 ).
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise added a comment.
I'm not sure how you would see script modifications via the user-agent. You can basically only send the SHA1 (or the first N nibbles) for that. Otherwise you'd need a database with all the SHA1 hashes of the scripts. But then it's probably better if it is done not via the `__version__` but by manually generating the hash and comparing it to the database (as `__version__` doesn't change when you edit it locally without committing it (or similar)).
For versioning of scripts I'm not sure. There is no sequence so you don't know if `a311a20ff092847c90f5064bd1c792487b75cb46` is newer or older than `f86e43093a4660aaea2b71d5a6694278d79ef656`. And then again there is afaik no “reverse search” to determine the version via the SHA1 has (but git could be configured to use the git hash).
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a blocked task: T85328: PEP-8 compliance.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added subscribers: hashar, Legoktm. jayvdb added a comment.
It appears a new version of pep8 has been deployed, and jenkins is rightly complaining about ... everything! https://integration.wikimedia.org/ci/job/pywikibot-core-tox-flake8/4758/cons...
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
Legoktm added a comment.
I don't think anything special was deployed on the WMF/CI side, just that the package was pushed to pypi, so tox downloaded the new version which is causing the failures..?
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: Legoktm Cc: Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
gerritbot added a subscriber: gerritbot. gerritbot added a project: Patch-For-Review. gerritbot added a comment.
Change 189168 had a related patch set uploaded (by John Vandenberg): Disable pep8 E402
https://gerrit.wikimedia.org/r/189168
https://phabricator.wikimedia.org/tag/patch-for-review/
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: gerritbot Cc: gerritbot, Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a comment.
In https://phabricator.wikimedia.org/T87409#1022409, @Legoktm wrote:
I don't think anything special was deployed on the WMF/CI side, just that the package was pushed to pypi, so tox downloaded the new version which is causing the failures..?
Hmm. on my Ubuntu and Fedora Core, these packages managed by the host ... not tox ... and I've checked that neither platform has already upgraded their packages for pep8. There is a tox setting to control that. Now when I run tox, I see
$ tox ERROR: unknown environment 'flake83'
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: gerritbot, Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
Ricordisamoa added a subscriber: Ricordisamoa.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: Ricordisamoa Cc: Ricordisamoa, gerritbot, Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
XZise added a comment.
On my Fedora 20 system I don't get the new errors.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: XZise Cc: Ricordisamoa, gerritbot, Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
gerritbot added a comment.
Change 189168 merged by jenkins-bot: Disable new pep8 v1.6 rules E402 and E731
https://gerrit.wikimedia.org/r/189168
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: gerritbot Cc: Ricordisamoa, gerritbot, Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
jayvdb added a comment.
As a start, we can at least remove `__version__` from the tests.
TASK DETAIL https://phabricator.wikimedia.org/T87409
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Ricordisamoa, gerritbot, Legoktm, hashar, Ladsgroup, Xqt, XZise, valhallasw, jayvdb, Aklapper, pywikipedia-bugs
pywikipedia-bugs@lists.wikimedia.org