Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 557148 - sys-apps/portage: repoman: Improve the commit message for git
Summary: sys-apps/portage: repoman: Improve the commit message for git
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Repoman (show other bugs)
Hardware: All Linux
: Normal enhancement (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: InVCS
Depends on:
Blocks: portage-2.3.0
  Show dependency tree
 
Reported: 2015-08-09 16:38 UTC by Julian Ospald
Modified: 2017-03-16 01:29 UTC (History)
5 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Ospald 2015-08-09 16:38:52 UTC
Repoman should add the commit message format guidelines to the template commit message as a comment at the very bottom, when you run plain "repoman commit".

https://wiki.gentoo.org/wiki/Gentoo_git_workflow#Commit_Message_Format

In addition, it should have basic functionality to add "foo/bar: " to the first line of the template commit message if you are in the ebuild directory "foo/bar". This will rule out a lot of commit message errors and make life easier especially for arch teams.
Comment 1 Agostino Sarubbo gentoo-dev 2015-08-10 07:50:16 UTC
Change the summary to be more visible for who search.
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-08-10 14:48:12 UTC
I've added prepending category and package as appropriate.
Comment 3 Agostino Sarubbo gentoo-dev 2015-08-11 07:12:49 UTC
Great, could we have this patch in the tree?
Thanks.
Comment 4 Cynede (RETIRED) gentoo-dev 2015-08-12 08:09:29 UTC
it will not break bug title if I also will mention suggestion to add `sign off` to commit message after portage version

Idea about it also was mentioned here: https://wiki.gentoo.org/wiki/Talk:Gentoo_git_workflow
Comment 5 Agostino Sarubbo gentoo-dev 2015-08-12 08:14:44 UTC
(In reply to Michał Górny from comment #2)
> I've added prepending category and package as appropriate.

It does not work for me:

commit 11b5619ffd193670842bd378a9237aed01e27a2a
Author: Agostino Sarubbo <ago@gentoo.org>
Date:   Wed Aug 12 10:10:38 2015 +0200

    Stable for amd64, wrt bug #557342
    
    Package-Manager: portage-2.2.20.1
    RepoMan-Options: --include-arches="amd64"


I'm available on irc for double-check.
Comment 6 Mikle Kolyada archtester Gentoo Infrastructure gentoo-dev Security 2015-08-12 10:57:48 UTC
(In reply to Agostino Sarubbo from comment #5)
> (In reply to Michał Górny from comment #2)
> > I've added prepending category and package as appropriate.
> 
> It does not work for me:
> 
> commit 11b5619ffd193670842bd378a9237aed01e27a2a
> Author: Agostino Sarubbo <ago@gentoo.org>
> Date:   Wed Aug 12 10:10:38 2015 +0200
> 
>     Stable for amd64, wrt bug #557342
>     
>     Package-Manager: portage-2.2.20.1
>     RepoMan-Options: --include-arches="amd64"
> 
> 
> I'm available on irc for double-check.

Wrong, in general it works. But only if you call 'repoman commit' without message option, then it opens ${EDITOR} (as usual), with ${category}/${pn} template in there.
Comment 7 Brian Dolbec gentoo-dev 2015-08-12 12:55:03 UTC
Released in portage-2.2.20.1

Does it work for you?  it does for me...
Comment 8 Mikle Kolyada archtester Gentoo Infrastructure gentoo-dev Security 2015-08-12 12:58:27 UTC
(In reply to Brian Dolbec from comment #7)
> Released in portage-2.2.20.1
> 
> Does it work for you?  it does for me...

it seems that ago wanted to get atom from repoman commit -m "<>", rather than call repoman commit without opts.
Comment 9 Julian Ospald 2015-08-12 13:04:14 UTC
(In reply to Mikle Kolyada from comment #8)
> (In reply to Brian Dolbec from comment #7)
> > Released in portage-2.2.20.1
> > 
> > Does it work for you?  it does for me...
> 
> it seems that ago wanted to get atom from repoman commit -m "<>", rather
> than call repoman commit without opts.

I've already told him that he can use the shell to make that easier:

in bashrc e.g.:

cat_pn() {
	awk -F/ '{print $(NF-1) "/" $(NF)}' <<<"${PWD}"
}

and then from within the ebuild directories:

repoman commit -m "$(cat_pn): stable for amd64"
Comment 10 Brian Dolbec gentoo-dev 2015-08-12 13:13:11 UTC
ah, well, adding -m "foo" implies a fully prepared commit message.

I doubt think there is a way to pre-fill that that will show.  It is impossible for a command to know what to put in an option before the command has run.  So realistically, all that can be done at that point is prepend it blindly.
I suppose the code could check for it in the first part of the string, but that is still subject to the string not having a typo in it which prevents it matching.

I'll leave this open for now.

If you decide you need it prepended to the -m "message", that can wait for the 2.2.21 release coming out soon.

If not you can close it as resolved/fixed
Comment 11 Julian Ospald 2015-08-12 13:18:37 UTC
(In reply to Brian Dolbec from comment #10)
> 
> If not you can close it as resolved/fixed

The other part of the bug report has not been resolved or discussed:
Add a (probably stripped down, only the most important points) version of the commit message policy to the template as a comment at the very bottom (below the repoman warnings).
Comment 12 Brian Dolbec gentoo-dev 2015-08-12 15:21:25 UTC
yeah, I'll see about adding a template system to repoman that won't require code changes (mostly) when the need for a change in the template is needed. It'll be a separate download.

But that will have to wait for the rewrite branch to land in main portage repo.
Comment 13 Agostino Sarubbo gentoo-dev 2015-08-12 16:12:24 UTC
(In reply to Julian Ospald (hasufell) from comment #9)
> (In reply to Mikle Kolyada from comment #8)
> > (In reply to Brian Dolbec from comment #7)
> > > Released in portage-2.2.20.1
> > > 
> > > Does it work for you?  it does for me...
> > 
> > it seems that ago wanted to get atom from repoman commit -m "<>", rather
> > than call repoman commit without opts.
> 
> I've already told him that he can use the shell to make that easier:

I already told you that I'm not committing manually and is not immediately change it in the script I use.


Anyway, I guess the main goal should be check if is present the prefix at the begin of the commit message, if not, add it.
Comment 14 Julian Ospald 2015-08-12 16:19:09 UTC
(In reply to Agostino Sarubbo from comment #13)
> (In reply to Julian Ospald (hasufell) from comment #9)
> > (In reply to Mikle Kolyada from comment #8)
> > > (In reply to Brian Dolbec from comment #7)
> > > > Released in portage-2.2.20.1
> > > > 
> > > > Does it work for you?  it does for me...
> > > 
> > > it seems that ago wanted to get atom from repoman commit -m "<>", rather
> > > than call repoman commit without opts.
> > 
> > I've already told him that he can use the shell to make that easier:
> 
> I already told you that I'm not committing manually and is not immediately
> change it in the script I use.
> 

Then add that bash function to your script? What's the problem?
Comment 15 Pacho Ramos gentoo-dev 2015-09-02 15:00:30 UTC
(In reply to Brian Dolbec from comment #10)
[...]
> If you decide you need it prepended to the -m "message", that can wait for
> the 2.2.21 release coming out soon.
> 
> If not you can close it as resolved/fixed

Well, I also vote for appending the ${CATEGORY}/${PN} at the start of the message, otherwise it's really easy to forget to add it and also, per current guidelines, not appending it on every "commit" is highly undesirable. What is the problem then on adding it at the start of the message always?

Thanks :)
Comment 16 Mike Gilbert gentoo-dev 2015-09-02 16:02:52 UTC
(In reply to Pacho Ramos from comment #15)

Some of us already have scripts which correctly assume that -m takes the entire commit message as its argument.

If this behavior is to change, it needs to be implemented as a separate option.
Comment 17 Brian Dolbec gentoo-dev 2017-03-16 01:29:18 UTC
Released in repoman-2.3.0 and later.