On Sat, 17 Feb 2018 13:18:03 +0000 "M. J. Everitt" <m.j.ever...@iee.org> wrote:
> On 17/02/18 12:56, Michał Górny wrote: > > Add a check for common mistakes in commit messages. For now, it is > > pretty rough and works only for -m/-F. It will be extended to work > > in the interactive mode in the future. > > --- > > repoman/pym/repoman/actions.py | 74 > > ++++++++++++++++++++++++- > > repoman/pym/repoman/tests/commit/__init__.py | 2 + > > repoman/pym/repoman/tests/commit/__test__.py | 1 + > > repoman/pym/repoman/tests/simple/test_simple.py | 8 +-- 4 files > > changed, 80 insertions(+), 5 deletions(-) create mode 100644 > > repoman/pym/repoman/tests/commit/__init__.py create mode 100644 > > repoman/pym/repoman/tests/commit/__test__.py > > > > diff --git a/repoman/pym/repoman/actions.py > > b/repoman/pym/repoman/actions.py index b76a6e466..91603865c 100644 > > --- a/repoman/pym/repoman/actions.py > > +++ b/repoman/pym/repoman/actions.py > > @@ -119,7 +119,16 @@ class Actions(object): > > if commitmessage[:9].lower() in ("cat/pkg: > > ",): commitmessage = self.msg_prefix() + commitmessage[9:] > > > > - if not commitmessage or not commitmessage.strip(): > > + if commitmessage is not None and > > commitmessage.strip(): > > + res, expl = > > self.verify_commit_message(commitmessage) > > + if not res: > > + print(bad("RepoMan does not like > > your commit message:")) > > + print(expl) > > + if self.options.force: > > + print('(but proceeding due > > to --force)') > > + else: > > + sys.exit(1) > > + else: > > commitmessage = > > self.get_new_commit_message(qa_output) > > commitmessage = commitmessage.rstrip() > > @@ -585,3 +594,66 @@ class Actions(object): > > print("* no commit message? aborting > > commit.") sys.exit(1) > > return commitmessage > > + > > + footer_re = re.compile(r'^[\w-]+:') > > + > > + @classmethod > > + def verify_commit_message(cls, msg): > > + """ > > + Check whether the message roughly complies with > > GLEP66. Returns > > + (True, None) if it does, (False, <explanation>) if > > it does not. > > + """ > > + > > + problems = [] > > + paras = msg.strip().split('\n\n') > > + summary = paras.pop(0) > > + > > + if ':' not in summary: > > + problems.append('summary line must start > > with a logical unit name, e.g. "cat/pkg:"') > > + if '\n' in summary.strip(): > > + problems.append('commit message must start > > with a *single* line of summary, followed by empty line') > > + # accept 69 overall or unit+50, in case of very > > long package names > > + elif len(summary.strip()) > 69 and > > len(summary.split(':', 1)[-1]) > 50: > > + problems.append('summary line is too long > > (max 69 characters)') + > > + multiple_footers = False > > + gentoo_bug_used = False > > + bug_closes_without_url = False > > + body_too_long = False > > + > > + found_footer = False > > + for p in paras: > > + lines = p.splitlines() > > + # if all lines look like footer material, > > we assume it's footer > > + # else, we assume it's body text > > + if all(cls.footer_re.match(l) for l in > > lines if l.strip()): > > + # multiple footer-like blocks? > > + if found_footer: > > + multiple_footers = True > > + found_footer = True > > + for l in lines: > > + if > > l.startswith('Gentoo-Bug'): > > + gentoo_bug_used = > > True > > + elif l.startswith('Bug:') > > or l.startswith('Closes:'): > > + if 'http://' not > > in l and 'https://' not in l: > > + > > bug_closes_without_url = True > > + else: > > + for l in lines: > > + # we recommend wrapping at > > 72 but accept up to 80; > > + # do not complain if we > > have single word (usually > > + # it means very long URL) > > + if len(l.strip()) > 80 and > > len(l.split()) > 1: > > + body_too_long = > > True + > > + if multiple_footers: > > + problems.append('multiple footer-style > > blocks found, please combine them into one') > > + if gentoo_bug_used: > > + problems.append('please replace Gentoo-Bug > > with GLEP 66-compliant Bug/Closes') > > + if bug_closes_without_url: > > + problems.append('Bug/Closes footers > > require full URL') > > + if body_too_long: > > + problems.append('body lines should be > > wrapped at 72 (max 80) characters') + > > + if problems: > > + return (False, '\n'.join('- %s' % x for x > > in problems)) > > + return (True, None) > > diff --git a/repoman/pym/repoman/tests/commit/__init__.py > > b/repoman/pym/repoman/tests/commit/__init__.py new file mode 100644 > > index 000000000..d74fd94a7 > > --- /dev/null > > +++ b/repoman/pym/repoman/tests/commit/__init__.py > > @@ -0,0 +1,2 @@ > > +# Copyright 2011-2018 Gentoo Foundation > > +# Distributed under the terms of the GNU General Public License v2 > > diff --git a/repoman/pym/repoman/tests/commit/__test__.py > > b/repoman/pym/repoman/tests/commit/__test__.py new file mode 100644 > > index 000000000..8b1378917 > > --- /dev/null > > +++ b/repoman/pym/repoman/tests/commit/__test__.py > > @@ -0,0 +1 @@ > > + > > diff --git a/repoman/pym/repoman/tests/simple/test_simple.py > > b/repoman/pym/repoman/tests/simple/test_simple.py index > > a24e0d5a3..3d7a70ad0 100644 --- > > a/repoman/pym/repoman/tests/simple/test_simple.py +++ > > b/repoman/pym/repoman/tests/simple/test_simple.py @@ -1,4 +1,4 @@ > > -# Copyright 2011-2015 Gentoo Foundation > > +# Copyright 2011-2018 Gentoo Foundation > > # Distributed under the terms of the GNU General Public License v2 > > > > import subprocess > > @@ -194,13 +194,13 @@ class SimpleRepomanTestCase(TestCase): > > ("", repoman_cmd + ("full", "-d")), > > ("", cp_cmd + (test_ebuild, > > test_ebuild[:-8] + "2.ebuild")), ("", git_cmd + ("add", > > test_ebuild[:-8] + "2.ebuild")), > > - ("", repoman_cmd + ("commit", "-m", "bump > > to version 2")), > > + ("", repoman_cmd + ("commit", "-m", > > "cat/pkg: bump to version 2")), ("", cp_cmd + (test_ebuild, > > test_ebuild[:-8] + "3.ebuild")), ("", git_cmd + ("add", > > test_ebuild[:-8] + "3.ebuild")), > > - ("dev-libs", repoman_cmd + ("commit", > > "-m", "bump to version 3")), > > + ("dev-libs", repoman_cmd + ("commit", > > "-m", "cat/pkg: bump to version 3")), ("", cp_cmd + (test_ebuild, > > test_ebuild[:-8] + "4.ebuild")), ("", git_cmd + ("add", > > test_ebuild[:-8] + "4.ebuild")), > > - ("dev-libs/A", repoman_cmd + ("commit", > > "-m", "bump to version 4")), > > + ("dev-libs/A", repoman_cmd + ("commit", > > "-m", "cat/pkg: bump to version 4")), ) > > > > env = { > Might I suggest breaking out checks into a separate module? I think > that hard-coding it all is likely to become a pain as time goes on, > more checks get added, etc ... > So, you would like the commit message checks to be a plug-in system like the ebuild checks are now. Yes that is possible, that would also allow overlays and sub-distros to customize it to their liking. But you do realize it would be a module system with only the one plug-in. It would add a slight amount of overhead and subsequently a tiny bit more time. Although it is not much as I remember when I first developed the plug-in system for emaint. I will consider that for the stage3 work in progress, but for the master branch of repoman, the hard-coded version above would be fine. -- Brian Dolbec <dolsen>
pgprVkLpLeUb3.pgp
Description: OpenPGP digital signature