[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
gerritbot added a comment. Change 368793 merged by jenkins-bot: [pywikibot/core@master] tox.ini: Check commit message guidelines using commit-message-validator https://gerrit.wikimedia.org/r/368793TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: gerritbotCc: zhuyifei1999, jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, Lordiis, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, Magul, Tbscho, MayS, Lewizho99, Mdupont, JJMC89, Maathavan, Avicennasis, Dalba, Masti, Alchimista, Rxy___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
gerritbot added a comment. Change 368793 had a related patch set uploaded (by Dalba; owner: Dalba): [pywikibot/core@master] tox.ini: Check commit message guidelines using commit-message-validator https://gerrit.wikimedia.org/r/368793TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: gerritbotCc: zhuyifei1999, jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, Magul, Tbscho, MayS, Mdupont, JJMC89, Avicennasis, Dalba, Masti, Alchimista, Rxy___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
hashar added a comment. Ops asked for some kind of commit message validation for operations/puppet. I went with adding a new env in tox which installs and run commit-message-validator from Pypi. https://gerrit.wikimedia.org/r/#/c/365609/3/tox.ini So it should be straightforward for pywikibot and any repository already relying on a job running tox. For other repositories, we had the idea of crafting a common job that would be attached on every single repos. Maybe we can add the existing job everywhere.TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: hasharCc: zhuyifei1999, jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, Magul, Tbscho, MayS, Mdupont, JJMC89, Avicennasis, Dalba, Masti, Alchimista, Rxy___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
Jdforrester-WMF added a comment. Two months later, with no issues, would it be possible to put it everywhere in non-voting mode and make it voting for (at least) VE-MW and maybe a couple of others?TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Jdforrester-WMFCc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, MayS, Lewizho99, Mdupont, JJMC89, Maathavan, Alchimista, Jay8g___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
Legoktm added a comment. This is now running against all mediawiki/extensions/VisualEditor commits in non-voting mode.TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: LegoktmCc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, Lewizho99, Mdupont, JJMC89, Maathavan, Jay8g___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
gerritbot added a comment. Change 303724 merged by Legoktm: Initial import from integration/config, set up packaging https://gerrit.wikimedia.org/r/303724TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: gerritbotCc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, Lewizho99, Mdupont, JJMC89, Maathavan, Jay8g___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
gerritbot added a comment. Change 303724 had a related patch set uploaded (by Legoktm): Initial import from integration/config, set up packaging https://gerrit.wikimedia.org/r/303724TASK DETAILhttps://phabricator.wikimedia.org/T109119EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: gerritbotCc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, Lewizho99, Mdupont, JJMC89, Maathavan, Jay8g___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
XZise added a comment. Please note that my paste hasn't been implemented and I haven't got around to port my code to the implemented variant. And checking if a commit hash has been merged has the problem that without SSH (@hashar do you know if it's available for the script?) we can only check if a hash has been merged, but it might be that the hash is for another repository if it's not merged. With SSH we could query gerrit to check if it's a future patch. Also what do you want with “Conflicts”? Suggest more checks (certainly possible) or are you concerned that the current parser doesn't allow it (as it doesn't allow any other `Foo: Bar` fields in the last paragraph)? In the latter case it shouldn't be a problem as the “Conflicts” section is separated by newlines. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: XZise Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
bd808 added a comment. In https://phabricator.wikimedia.org/T109119#1696489, @XZise wrote: > And checking if a commit hash has been merged has the problem that without > SSH (@hashar do you know if it's available for the script?) we can only check > if a hash has been merged, but it might be that the hash is for another > repository if it's not merged. With SSH we could query gerrit to check if > it's a future patch. Let's try hard not to tie this to Gerrit internals. I think that migration to Differential will happen sooner than many people imagine. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: bd808 Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
XZise added a comment. In https://phabricator.wikimedia.org/T109119#1697346, @bd808 wrote: > In https://phabricator.wikimedia.org/T109119#1696489, @XZise wrote: > > > And checking if a commit hash has been merged has the problem that without > > SSH (@hashar do you know if it's available for the script?) we can only > > check if a hash has been merged, but it might be that the hash is for > > another repository if it's not merged. With SSH we could query gerrit to > > check if it's a future patch. > > > Let's try hard not to tie this to Gerrit internals. I think that migration to > Differential will happen sooner than many people imagine. Well that all depends on the implementation, doesn't it? I already have written a script to work with Gerrit so it's not much work to me. And then it could be “generic” and just be a function which returns whether a commit/changeid/D… reference hasn't been merged. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: XZise Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
bd808 added a comment. In https://phabricator.wikimedia.org/T109119#1696169, @jayvdb wrote: > One of the easy checks in the original task description is not yet > implemented: > > - There should be no I… references but instead always git commit hashes. So I should not be able to make commit messages to refer to uncommitted change sets (or a series of related changes cherry-picked to multiple branches)? Is this desired so that commit history information is self contained? I assume you would intend it to extend to D... references when we finally get switched to Differential instead of Gerrit. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: bd808 Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
XZise added a comment. In https://phabricator.wikimedia.org/T109119#1697336, @bd808 wrote: > In https://phabricator.wikimedia.org/T109119#1696169, @jayvdb wrote: > > > One of the easy checks in the original task description is not yet > > implemented: > > > > - There should be no I… references but instead always git commit hashes. > > > So I should not be able to make commit messages to refer to uncommitted > change sets (or a series of related changes cherry-picked to multiple > branches)? Is this desired so that commit history information is self > contained? I personally prefer that self containment and don't really see when a patch needs to reference a future patch. That future patch may get abandoned to the feature from that patch may get implemented on another patch. Afaik patches should only reference in the past. And in which case should a patch mention that it's going to be cherry picked in another branch? The same applies to the above, what happens if that cherry pick does not happen? And in any case that cherry pick references to the original patch it comes from. And what happens if the patch gets cherry picked into other branches and are thus not present in the original patch's message. Mentioning git commit hashes has the big advantage that you don't have to resolve them as I mention in the original post. In https://phabricator.wikimedia.org/T109119#1697336, @bd808 wrote: > I assume you would intend it to extend to D... references when we finally get > switched to Differential instead of Gerrit. I'm not sure how Differential actually works but alone for the reason I mention in the original post, that you need to resolve that manually or need Differential, it should apply to that too. Please note that, as John mentioned, the current implementation does not pose that limit, so even if it would be enabled it wouldn't be necessary. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: XZise Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
jayvdb added a comment. Maybe we can query https://phabricator.wikimedia.org/tag/phabricator/ for the sha1, which would identify which repository it is merged into. For referencing commits in repos not in https://phabricator.wikimedia.org/tag/phabricator/, we could support github slugs (which will auto link when the commit is merged and replicated to github) For all other cases, a URL can be used instead of, or as a prefix to, a sha1. Re `Conflicts:`, I am wondering what other repos are practising. i.e. when should they be kept vs removed? TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: jayvdb Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
JanZerebecki added a comment. https access should be possible from Jenkins nodepool slaves, because it will be needed for npm and composer. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: JanZerebecki Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
jayvdb added a comment. In https://phabricator.wikimedia.org/T109119#1697336, @bd808 wrote: > In https://phabricator.wikimedia.org/T109119#1696169, @jayvdb wrote: > > > One of the easy checks in the original task description is not yet > > implemented: > > > > - There should be no I… references but instead always git commit hashes. > > > So I should not be able to make commit messages to refer to uncommitted > change sets (or a series of related changes cherry-picked to multiple > branches)? Is this desired so that commit history information is self > contained? I assume you would intend it to extend to D... references when we > finally get switched to Differential instead of Gerrit. @bd808, we would still put uncommitted sha1/whatever in commit messages, due to dependencies, but the patch would be -1'd until the dependencies are committed. The advantage is that with a checker, we can be confident that we'll be reminded to update git hashes if the dependencies were revised during code review and the referenced sha1 became obsolete. It also provides a helpful reminder that some retesting might be needed if the dependency changed before it was committed. The disadvantage is that the -1 can reduce the amount of code review the patch will see as it will be filtered out of some queries. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: jayvdb Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
jayvdb added a subscriber: jayvdb. jayvdb added a comment. One of the easy checks in the original task description is not yet implemented: - There should be no I… references but instead always git commit hashes. That is ~ https://phabricator.wikimedia.org/P1881 line 186 Then the non-trivial follow-on from that in the original task description is yet to be implemented: - Commit hashes which point to commits in should only point to past (merged) commits. It would also be good to introduce a strict syntax for referring to commits in other repos. And another hard one that I would like is a standardised way of referring to regressions, with a reference to the commit where the regression occurred. This will help automated identification of changes which need to be cherry-picked to other branches based on whether the regression commit exists in the branch. It would be good to standardise the list of other rfc822 headers which are allowed in the message tail. e.g. Signed-off-by , Original-author:/Co-author: . Do other repos normally allow 'Conflicts:\nfoo' which git automatically adds? TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: jayvdb Cc: jayvdb, Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
Jdforrester-WMF added a subscriber: Jdforrester-WMF. Jdforrester-WMF added a comment. I volunteer the VE repos for testing this before we put it live everywhere. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Jdforrester-WMF Cc: Jdforrester-WMF, JanZerebecki, Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
gerritbot added a comment. Change 237719 merged by jenkins-bot: Add commit-message-validator.py tool https://gerrit.wikimedia.org/r/237719 TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: Legoktm, bd808, gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
gerritbot added a subscriber: gerritbot. gerritbot added a comment. Change 237719 had a related patch set uploaded (by Legoktm): Add commit-message-validator.py tool https://gerrit.wikimedia.org/r/237719 TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: gerritbot, hashar, greg, Aklapper, pywikibot-bugs-list, XZise, jayvdb ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
XZise added a comment. Okay I updated https://phabricator.wikimedia.org/P1881 to work more like pep8/flake8 as they are returning the error codes now in the order they appear in the message. It's not doing a SSH request if these two codes requiring it are ignored anyway. I added help text and a few other codes which check that the commit hash lengths are all equal and don't use mixed upper/lower case. I'd appreciate feedback about other repos than pywikibot whether the rules are also apply to their repos and if there are additional rules. The code is not really perfect. At the moment M903 will be always replaced by M902 and M204 should check all commit hashes and not one… so if someone is using `1234aBcd` it should flagged as well as when `1234abcd` and `1234ABCD` is used within a message. Note that the codes 900 are only warnings so (unless enabled) won't cause a failure. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: XZise Cc: greg, Aklapper, pywikibot-bugs-list, XZise, jayvdb, hashar, Malyacko ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
XZise added a comment. Okay afaik there is nothing pywikibot specific. So if others projects want they could use it too or are the any projects which handle that differently? Part of the rules are also applying Gerrit/Commit message guidelines https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines. TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: XZise Cc: greg, Aklapper, pywikibot-bugs-list, XZise, jayvdb, hashar, Malyacko ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
greg added a subscriber: greg. greg added a comment. I'm pro this being a generic job for all projects (iow: I don't think making per-project jobs for this is sustainable/wise). TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: greg Cc: greg, Aklapper, pywikibot-bugs-list, XZise, jayvdb, hashar, Malyacko ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
[Pywikipedia-bugs] [Maniphest] [Commented On] T109119: Check the style of the commit message
XZise added a comment. Also “Commit hashes which point to commits in pywikibot should only point to past (merged) commits.” is also applicable to any other project in gerrit. And in we could use gerrit to determine if that commit hash has been merged even if it's in a different commit. Unfortunately I don't know how Jenkins run but I think I'd be ale to write a short script checking most of them with access to SSH (to run queries against Gerrit) and git (to get the commit message). TASK DETAIL https://phabricator.wikimedia.org/T109119 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: XZise Cc: greg, Aklapper, pywikibot-bugs-list, XZise, jayvdb, hashar, Malyacko ___ pywikibot-bugs mailing list pywikibot-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
