Re: [PATCH v4 06/31] python: add commit-per-subsystem.py

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Oct 07, 2019 at 06:16:51PM +0200, Cornelia Huck wrote:
> On Mon, 7 Oct 2019 16:10:02 +0000
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> 
> > 07.10.2019 18:55, Cornelia Huck wrote:
> > > On Tue,  1 Oct 2019 18:52:54 +0300
> > > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> 
> > >> +def git_add(pattern):
> > >> +    subprocess.run(['git', 'add', pattern])
> > >> +
> > >> +
> > >> +def git_commit(msg):
> > >> +    subprocess.run(['git', 'commit', '-m', msg], capture_output=True)
> > >> +
> > >> +
> > >> +maintainers = sys.argv[1]
> > >> +message = sys.argv[2].strip()
> > >> +
> > >> +subsystem = None
> > >> +
> > >> +shortnames = {
> > >> +    'Block layer core': 'block',
> > >> +    'ARM cores': 'arm',
> > >> +    'Network Block Device (NBD)': 'nbd',
> > >> +    'Command line option argument parsing': 'cmdline',
> > >> +    'Character device backends': 'chardev',
> > >> +    'S390 general architecture support': 's390'
> > >> +}
> > >> +
> > >> +
> > >> +def commit():
> > >> +    if subsystem:
> > >> +        msg = subsystem
> > >> +        if msg in shortnames:
> > >> +            msg = shortnames[msg]
> > >> +        msg += ': ' + message
> > >> +        git_commit(msg)
> > >> +
> > >> +
> > >> +with open(maintainers) as f:
> > >> +    for line in f:
> > >> +        line = line.rstrip()
> > >> +        if not line:
> > >> +            continue
> > >> +        if len(line) >= 2 and line[1] == ':':
> > >> +            if line[0] == 'F' and line[3:] not in ['*', '*/']:
> > >> +                git_add(line[3:])
> > >> +        else:
> > >> +            # new subsystem start
> > >> +            commit()
> > >> +
> > >> +            subsystem = line
> > >> +
> > >> +commit()  
> > > 
> > > Hm... I'm not sure about the purpose of this script (and my python is
> > > rather weak)... is this supposed to collect all changes covered by a
> > > subsystem F: pattern into one patch?  
> > 
> > Yes
> > 
> > > If so, what happens to files
> > > covered by multiple sections?
> > >   
> > 
> > Hmm, they just go to the first of these sections, mentioned in MAINTAINERS.
> > Is it bad I don't know, but I tried to automate it somehow. Anyway, I myself
> > can't have better idea about how to organize patches to the subsystems which
> > I don't know.
> > 
> 
> Yeah, that is a problem I don't have a solution for, either.
> 
> But the script should probably get at least a comment about its
> intended purpose and limitations? We don't really want people to start
> using it blindly.

Is this really a common enough problem to even justify having the
script to start with ?

It looks like its only really usable in the case where the changes
to each subsystem are totally self-contained, otherwise you'll get
git bisect failures. The user still has to go back and edit each
commit here to fill in a useful commit messages. It doesn't seem
to save much effort over 'git add -u sub/dir/ && git commit -s'
which is what I'd typically do for grouping changes that are
spread across the tree.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|