| Submitter | Gregory Szorc |
|---|---|
| Date | Feb. 3, 2017, 7:56 a.m. |
| Message ID | <5fe78521b9cb553b9a7c.1486108589@ubuntu-vm-main> |
| Download | mbox | patch |
| Permalink | /patch/18308/ |
| State | Deferred |
| Headers | show |
Comments
Gregory Szorc a écrit : > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1486108530 28800 > # Thu Feb 02 23:55:30 2017 -0800 > # Node ID 5fe78521b9cb553b9a7c6bd4d96576a35b8d3517 > # Parent abf029200e198878a4576a87e095bd8d77d9cea9 > contrib: script to generate release notes from commit messages [snip] > diff --git a/tests/test-generate-release-notes.t b/tests/test-generate-release-notes.t > new file mode 100644 > --- /dev/null > +++ b/tests/test-generate-release-notes.t > @@ -0,0 +1,145 @@ > +Create a fake repo with a relnotes directory and commits > + > + $ hg init repo0 > + $ export FAKESRCDIR=$TESTTMP/repo0 > + $ cd repo0 > + $ mkdir relnotes > + $ touch relnotes/4.1.rst > + $ hg commit -A -m 'add relnotes 4.1' > + adding relnotes/4.1.rst > + > +4.1.rst should be used for relnotes if only available file > + > + $ $TESTDIR/../contrib/generate-release-notes > + updating $TESTTMP/repo0/relnotes/4.1.rst with content from 1 changesets > + > +4.1.1.rst is used over 4.1.rst > + > + $ touch relnotes/4.1.1.rst > + $ hg commit -A -m 'add relnotes 4.1.1' > + adding relnotes/4.1.1.rst > + $ $TESTDIR/../contrib/generate-release-notes > + updating $TESTTMP/repo0/relnotes/4.1.1.rst with content from 2 changesets > + > +4.2 is used over 4.1.1 > + > + $ touch relnotes/4.2.rst > + $ hg commit -A -m 'add relnotes 4.2' > + adding relnotes/4.2.rst > + $ $TESTDIR/../contrib/generate-release-notes > + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 3 changesets > + > +A fix with a single line is documented with .. fix:: > + > + $ touch fix1 > + $ hg commit -A -l - << EOF > + > summary line > + > > + > .. fix:: > + > > + > this is a simple fix with a single line > + > EOF I looks strange to have an rst block body that is not indented. Typically this is not valid rst as the "fix" directive has no content block here. So maybe we should consider indentation? That would also make it possible to have more text after a directive like: .. fix:: this is a simple fix with a single line more content not to be part of the release notes.
On Fri, Feb 3, 2017 at 12:40 AM, Denis Laxalde <denis.laxalde@logilab.fr> wrote: > Gregory Szorc a écrit : > >> # HG changeset patch >> # User Gregory Szorc <gregory.szorc@gmail.com> >> # Date 1486108530 28800 >> # Thu Feb 02 23:55:30 2017 -0800 >> # Node ID 5fe78521b9cb553b9a7c6bd4d96576a35b8d3517 >> # Parent abf029200e198878a4576a87e095bd8d77d9cea9 >> contrib: script to generate release notes from commit messages >> > > [snip] > > > diff --git a/tests/test-generate-release-notes.t >> b/tests/test-generate-release-notes.t >> new file mode 100644 >> --- /dev/null >> +++ b/tests/test-generate-release-notes.t >> @@ -0,0 +1,145 @@ >> +Create a fake repo with a relnotes directory and commits >> + >> + $ hg init repo0 >> + $ export FAKESRCDIR=$TESTTMP/repo0 >> + $ cd repo0 >> + $ mkdir relnotes >> + $ touch relnotes/4.1.rst >> + $ hg commit -A -m 'add relnotes 4.1' >> + adding relnotes/4.1.rst >> + >> +4.1.rst should be used for relnotes if only available file >> + >> + $ $TESTDIR/../contrib/generate-release-notes >> + updating $TESTTMP/repo0/relnotes/4.1.rst with content from 1 >> changesets >> + >> +4.1.1.rst is used over 4.1.rst >> + >> + $ touch relnotes/4.1.1.rst >> + $ hg commit -A -m 'add relnotes 4.1.1' >> + adding relnotes/4.1.1.rst >> + $ $TESTDIR/../contrib/generate-release-notes >> + updating $TESTTMP/repo0/relnotes/4.1.1.rst with content from 2 >> changesets >> + >> +4.2 is used over 4.1.1 >> + >> + $ touch relnotes/4.2.rst >> + $ hg commit -A -m 'add relnotes 4.2' >> + adding relnotes/4.2.rst >> + $ $TESTDIR/../contrib/generate-release-notes >> + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 3 >> changesets >> + >> +A fix with a single line is documented with .. fix:: >> + >> + $ touch fix1 >> + $ hg commit -A -l - << EOF >> + > summary line >> + > >> + > .. fix:: >> + > >> + > this is a simple fix with a single line >> + > EOF >> > > I looks strange to have an rst block body that is not indented. Typically > this is not valid rst as the "fix" directive has no content block here. So > maybe we should consider indentation? That would also make it possible to > have more text after a directive like: > > .. fix:: > > this is a simple fix with a single line > > more content not to be part of the release notes. > Yes, that is a good suggestion. It also makes the parser simpler since you can just look for indented lines. I'd still like positive feedback from others before I finish this patch: I don't want to sink more time in it before there is buy-in to this approach.
On Thu, Feb 02, 2017 at 11:56:29PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1486108530 28800 > # Thu Feb 02 23:55:30 2017 -0800 > # Node ID 5fe78521b9cb553b9a7c6bd4d96576a35b8d3517 > # Parent abf029200e198878a4576a87e095bd8d77d9cea9 > contrib: script to generate release notes from commit messages > > Per discussion on the mailing list, we want better release notes. > > This patch introduces a script for producing better release notes. Did you look at adapting towncrier from hawkowl on github at all? I'm sure it's git-only now, but maybe that's easy to adapt? (I suspect you did look, but wanted to be sure. That said, this ain't much code...) Overall, I think I'm pretty happy with this approach, though since I'm the one that brought it up I guess that's unsurprising. How does this handle the file being copy-edited mid-stream? That is, if I copy-edit one of the release note lines, will it get clobbered by future runs of the tool? As for who and when, if we don't expect much copyediting before release time, maybe the thing to do is to just run this as part of the release process and do the copyediting then? Or if we do, we could just have a bot do it on mercurial-scm.org as part of the accept process tooling. > diff --git a/contrib/generate-release-notes b/contrib/generate-release-notes > new file mode 100755 > --- /dev/null > +++ b/contrib/generate-release-notes > @@ -0,0 +1,270 @@ > + > +def unpublishedcommitmessages(): > + """Obtain commit messages for unpublished commits.""" Is it the intent that this only scrapes the log messages of non-public commits? I feel like maybe the approach should instead be to record a last-run version in the .rst file, and then we process everything in the repo that's a descendant of that or something? > + > + env = dict(os.environ) > + env['HGMODULEPOLICY'] = 'py' > + > + sep = str(uuid.uuid4()) > + > + args = [ > + 'hg', > + 'log', > + '-r', '::. and not public()', > + '-T', '{desc}%s' % sep > + ] > + > + p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, > + cwd=SRCDIR, env=env) > + out, err = p.communicate() > + res = p.wait() > + if res: > + raise Exception('error running `hg log`') > + > + return out.split(sep)[:-1] > +
Gregory Szorc <gregory.szorc@gmail.com> writes: > On Fri, Feb 3, 2017 at 12:40 AM, Denis Laxalde <denis.laxalde@logilab.fr> > wrote: > >> Gregory Szorc a écrit : >> >>> # HG changeset patch >>> # User Gregory Szorc <gregory.szorc@gmail.com> >>> # Date 1486108530 28800 >>> # Thu Feb 02 23:55:30 2017 -0800 >>> # Node ID 5fe78521b9cb553b9a7c6bd4d96576a35b8d3517 >>> # Parent abf029200e198878a4576a87e095bd8d77d9cea9 >>> contrib: script to generate release notes from commit messages >>> >> >> [snip] >> >> >> diff --git a/tests/test-generate-release-notes.t >>> b/tests/test-generate-release-notes.t >>> new file mode 100644 >>> --- /dev/null >>> +++ b/tests/test-generate-release-notes.t >>> @@ -0,0 +1,145 @@ >>> +Create a fake repo with a relnotes directory and commits >>> + >>> + $ hg init repo0 >>> + $ export FAKESRCDIR=$TESTTMP/repo0 >>> + $ cd repo0 >>> + $ mkdir relnotes >>> + $ touch relnotes/4.1.rst >>> + $ hg commit -A -m 'add relnotes 4.1' >>> + adding relnotes/4.1.rst >>> + >>> +4.1.rst should be used for relnotes if only available file >>> + >>> + $ $TESTDIR/../contrib/generate-release-notes >>> + updating $TESTTMP/repo0/relnotes/4.1.rst with content from 1 >>> changesets >>> + >>> +4.1.1.rst is used over 4.1.rst >>> + >>> + $ touch relnotes/4.1.1.rst >>> + $ hg commit -A -m 'add relnotes 4.1.1' >>> + adding relnotes/4.1.1.rst >>> + $ $TESTDIR/../contrib/generate-release-notes >>> + updating $TESTTMP/repo0/relnotes/4.1.1.rst with content from 2 >>> changesets >>> + >>> +4.2 is used over 4.1.1 >>> + >>> + $ touch relnotes/4.2.rst >>> + $ hg commit -A -m 'add relnotes 4.2' >>> + adding relnotes/4.2.rst >>> + $ $TESTDIR/../contrib/generate-release-notes >>> + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 3 >>> changesets >>> + >>> +A fix with a single line is documented with .. fix:: >>> + >>> + $ touch fix1 >>> + $ hg commit -A -l - << EOF >>> + > summary line >>> + > >>> + > .. fix:: >>> + > >>> + > this is a simple fix with a single line >>> + > EOF >>> >> >> I looks strange to have an rst block body that is not indented. Typically >> this is not valid rst as the "fix" directive has no content block here. So >> maybe we should consider indentation? That would also make it possible to >> have more text after a directive like: >> >> .. fix:: >> >> this is a simple fix with a single line >> >> more content not to be part of the release notes. >> > > Yes, that is a good suggestion. It also makes the parser simpler since you > can just look for indented lines. > > I'd still like positive feedback from others before I finish this patch: I > don't want to sink more time in it before there is buy-in to this approach. I like the idea of using mailing list (as a review of text) + commit message but as Sid mentioned, we will still need to copy/edit. It's inevitable since we're human. So, as a source of text, the commit message makes sense. But how will we solve copy editing?
Augie Fackler <raf@durin42.com> writes: > On Thu, Feb 02, 2017 at 11:56:29PM -0800, Gregory Szorc wrote: >> # HG changeset patch >> # User Gregory Szorc <gregory.szorc@gmail.com> >> # Date 1486108530 28800 >> # Thu Feb 02 23:55:30 2017 -0800 >> # Node ID 5fe78521b9cb553b9a7c6bd4d96576a35b8d3517 >> # Parent abf029200e198878a4576a87e095bd8d77d9cea9 >> contrib: script to generate release notes from commit messages >> >> Per discussion on the mailing list, we want better release notes. >> >> This patch introduces a script for producing better release notes. > > Did you look at adapting towncrier from hawkowl on github at all? I'm > sure it's git-only now, but maybe that's easy to adapt? > > (I suspect you did look, but wanted to be sure. That said, this ain't > much code...) > > Overall, I think I'm pretty happy with this approach, though since I'm > the one that brought it up I guess that's unsurprising. How does this > handle the file being copy-edited mid-stream? That is, if I copy-edit > one of the release note lines, will it get clobbered by future runs of > the tool? > > As for who and when, if we don't expect much copyediting before > release time, maybe the thing to do is to just run this as part of the > release process and do the copyediting then? Or if we do, we could > just have a bot do it on mercurial-scm.org as part of the accept > process tooling. We could also do some kind of hybrid approach of committing a file (or files) that apply a patch (or some kind of transformation) to the commit message. Just thinking out loud here.
Patch
diff --git a/contrib/generate-release-notes b/contrib/generate-release-notes new file mode 100755 --- /dev/null +++ b/contrib/generate-release-notes @@ -0,0 +1,270 @@ +#!/usr/bin/env python +# generate-release-notes -- populate release notes from commit messages +# +# Copyright 2017 Gregory Szorc <gregory.szorc@gmail.com> +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2 or any later version. + +""" +Release notes files live in ``relnotes/<version>.rst``. + +Release notes files have sections defined by ``SECTIONS`` below. + +If commit messages have lines beginning with ``.. <section>::``, the content +that follows is populated in that section of the release notes. + +The ``feature`` section is special in that it can have a title. e.g. +``.. feature:: this is a new feature``. This will get formatted as its +own sub-section in the release notes. + +""" + +from __future__ import absolute_import, print_function + +import os +import subprocess +import textwrap +import uuid + +# For testing. +if 'FAKESRCDIR' in os.environ: + SRCDIR = os.environ['FAKESRCDIR'] +else: + SRCDIR = os.path.normpath(os.path.join(os.path.dirname(__file__), '..')) + +# (header, internal, has sub-sections) +SECTIONS = [ + ('New Features', 'features', True,), + ('Backwards Compatibility Changes', 'bc', False), + ('Fixes', 'fix', False), + ('Performance Improvements', 'perf', False), + ('API Changes', 'api', False), +] + +def relnotespath(): + """Obtain the path to the current relnotes file. + + The current relnotes file is the highest versioned file in the relnotes + directory. + """ + files = os.listdir(os.path.join(SRCDIR, 'relnotes')) + assert all(f.endswith('.rst') for f in files) + versions = [map(int, f[:-4].split('.')) for f in files] + + latest = '.'.join(map(str, list(sorted(versions))[-1])) + + return os.path.join(SRCDIR, 'relnotes', '%s.rst' % latest) + +def unpublishedcommitmessages(): + """Obtain commit messages for unpublished commits.""" + + env = dict(os.environ) + env['HGMODULEPOLICY'] = 'py' + + sep = str(uuid.uuid4()) + + args = [ + 'hg', + 'log', + '-r', '::. and not public()', + '-T', '{desc}%s' % sep + ] + + p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + cwd=SRCDIR, env=env) + out, err = p.communicate() + res = p.wait() + if res: + raise Exception('error running `hg log`') + + return out.split(sep)[:-1] + +def parsenotes(path): + notes = {} + + for header, section, hassub in SECTIONS: + notes[section] = [] + + with open(path, 'rb') as fh: + lines = [l.rstrip() for l in fh] + + i = 0 + section = None + subsection = None + itemlines = [] + while i < len(lines): + line = lines[i] + + if not line: + # Flush existing entry. + if itemlines: + notes[section].append(itemlines) + itemlines = [] + i += 1 + continue + + # Section header. + if i + 1 < len(lines) and lines[i + 1].startswith('='): + section = None + subsection = None + for header, sec, hassub in SECTIONS: + if line == header: + section = sec + + if not section: + raise Exception('Unknown section in %s: %s' % (path, line)) + + i += 2 + continue + + # Sub-section header. + if i + 1 < len(lines) and lines[i + 1].startswith('-'): + subsection = line + + assert section + + # New item. + if line.startswith('*'): + # Flush existing entry. + if itemlines: + if subsection: + notes[section].append((subsection, itemlines)) + else: + notes[section].append(itemlines) + + itemlines = [] + + assert line.startswith('* ') + itemlines.append(line[2:]) + i += 1 + continue + + # Item continuation. + if line.startswith(' '): + itemlines.append(line[2:]) + i += 1 + continue + + raise Exception('unexpected content in release notes file: %s' % line) + + return notes + +def serializenotes(notes): + lines = [] + + for header, section, hassub in SECTIONS: + if not notes[section]: + continue + + lines.append(header) + lines.append('=' * len(header)) + lines.append('') + + for itemlines in notes[section]: + paralines = [] + initial = [True] + + def flush(): + if not paralines: + return + + if initial[0]: + indent = '* ' + initial[0] = False + else: + lines.append('') + indent = ' ' + + wrapper = textwrap.TextWrapper(initial_indent=indent, + subsequent_indent=' ', + width=78) + lines.extend(wrapper.wrap(' '.join(paralines))) + paralines[:] = [] + + + for line in itemlines: + if not line: + flush() + continue + + paralines.append(line) + + flush() + + lines.append('') + + return '\n'.join(lines) + +def updaterelnotes(): + path = relnotespath() + notes = parsenotes(path) + messages = unpublishedcommitmessages() + + print('updating %s with content from %d changesets' % (path, len(messages))) + + for message in messages: + lines = message.splitlines() + + state = { + 'header': None, + 'section': None, + 'lines': [], + } + + def flush(): + if not state['section']: + return + + lines = state['lines'] + while not lines[0]: + lines = lines[1:] + while not lines[-1]: + lines = lines[:-1] + + # We've already seen this release note. Ignore it. + if lines not in notes[state['section']]: + notes[state['section']].append(lines) + + state['header'] = None + state['section'] = None + state['lines'] = [] + + i = 0 + while i < len(lines): + line = lines[i] + issection = False + for header, section, hassub in SECTIONS: + if hassub: + if line.startswith('.. %s:: ' % section): + flush() + section = line[3:-2] + state['section'] = section + state['header'] = line[len(section) + 5:] + issection = True + break + elif line == '.. %s::' % section: + flush() + state['section'] = line[3:-2] + state['header'] = None + issection = True + break + + # Found a new section header. + if issection: + i += 1 + continue + + # Add this content to active section. + if state['section']: + state['lines'].append(line) + + i += 1 + + flush() + + with open(path, 'wb') as fh: + fh.write(serializenotes(notes)) + +if __name__ == '__main__': + updaterelnotes() diff --git a/tests/test-generate-release-notes.t b/tests/test-generate-release-notes.t new file mode 100644 --- /dev/null +++ b/tests/test-generate-release-notes.t @@ -0,0 +1,145 @@ +Create a fake repo with a relnotes directory and commits + + $ hg init repo0 + $ export FAKESRCDIR=$TESTTMP/repo0 + $ cd repo0 + $ mkdir relnotes + $ touch relnotes/4.1.rst + $ hg commit -A -m 'add relnotes 4.1' + adding relnotes/4.1.rst + +4.1.rst should be used for relnotes if only available file + + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.1.rst with content from 1 changesets + +4.1.1.rst is used over 4.1.rst + + $ touch relnotes/4.1.1.rst + $ hg commit -A -m 'add relnotes 4.1.1' + adding relnotes/4.1.1.rst + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.1.1.rst with content from 2 changesets + +4.2 is used over 4.1.1 + + $ touch relnotes/4.2.rst + $ hg commit -A -m 'add relnotes 4.2' + adding relnotes/4.2.rst + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 3 changesets + +A fix with a single line is documented with .. fix:: + + $ touch fix1 + $ hg commit -A -l - << EOF + > summary line + > + > .. fix:: + > + > this is a simple fix with a single line + > EOF + adding fix1 + + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 4 changesets + + $ cat relnotes/4.2.rst + Fixes + ===== + + * this is a simple fix with a single line + +Another fix is appended properly + + $ touch fix2 + $ hg commit -A -l - << EOF + > summary line + > + > .. fix:: + > + > this is another simple fix + > EOF + adding fix2 + + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 5 changesets + + $ cat relnotes/4.2.rst + Fixes + ===== + + * this is a simple fix with a single line + * this is another simple fix + +An entry with a long line is line wrapped + + $ touch fix3 + $ hg commit -A -l - << EOF + > summary line + > .. fix:: + > + > This is another fix. It has a long line that should wrap when written to the release notes file. + > EOF + adding fix3 + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 6 changesets + + $ cat relnotes/4.2.rst + Fixes + ===== + + * this is a simple fix with a single line + * this is another simple fix + * This is another fix. It has a long line that should wrap when written to the + release notes file. + +An entry with multiple lines is recorded properly + + $ touch fix4 + $ hg commit -A -l - << EOF + > summary line + > + > .. fix:: + > + > This is the first line of a fix. + > And the second line. + > + > And the line after a blank line. + > EOF + adding fix4 + + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 7 changesets + + $ cat relnotes/4.2.rst + Fixes + ===== + + * this is a simple fix with a single line + * this is another simple fix + * This is another fix. It has a long line that should wrap when written to the + release notes file. + * This is the first line of a fix. And the second line. + + And the line after a blank line. + +Generating again will preserve original content +TODO fix the parser + + $ $TESTDIR/../contrib/generate-release-notes + updating $TESTTMP/repo0/relnotes/4.2.rst with content from 7 changesets + $ cat relnotes/4.2.rst + Fixes + ===== + + * this is a simple fix with a single line + * this is another simple fix + * This is another fix. It has a long line that should wrap when written to the + release notes file. + * This is the first line of a fix. And the second line. + * This is another fix. It has a long line that should wrap when written to the + release notes file. + * This is the first line of a fix. And the second line. + + And the line after a blank line.