PR#2700: Make the pagure hook act as the person doing the push - pagure

pagure

#2700 Make the pagure hook act as the person doing the push

Merged 6 years ago by pingou. Opened 6 years ago by pingou.

pagure_hook_user into master

Add a small debugging statement

Pierre-Yves Chibon • 6 years ago

9404ddc

Make the pagure hook act as the person doing the push

Pierre-Yves Chibon • 6 years ago

67c8211

pagure/hooks/files/pagure_hook.py

file modified

+12 -5

		`@@ -78,12 +78,15 @@`
		`comment = ''' Commit [%s](%s) relates to this ticket''' % (`
		`commitid[:8], url)`

		`+ user = os.environ.get(`
		`+ 'GL_USER', pagure.lib.git.get_author_email(commitid, abspath))`
		`+`
		`try:`
		`pagure.lib.add_issue_comment(`
		`pagure.SESSION,`
		`issue=issue,`
		`comment=comment,`
		`- user=pagure.lib.git.get_author_email(commitid, abspath),`
		`+ user=user,`
		`ticketfolder=pagure.APP.config['TICKETS_FOLDER'],`
		`)`
		`pagure.SESSION.commit()`
		`@@ -110,13 +113,16 @@`
		`comment = ''' Commit [%s](%s) fixes this %s''' % (`
		`commitid[:8], url, relation.isa)`

		`+ user = os.environ.get(`
		`+ 'GL_USER', pagure.lib.git.get_author_email(commitid, abspath))`
bowlofeggs commented 6 years ago I wonder whether this would make a good function since it's used twice in this patch.
pingou commented 6 years ago For the moment it's pretty straight forward so I'll keep this here but if it's needed in more places it may make sense to move it to a function indeed.
		`+`
		`try:`
		`if relation.isa == 'issue':`
		`pagure.lib.add_issue_comment(`
		`pagure.SESSION,`
		`issue=relation,`
		`comment=comment,`
		`- user=pagure.lib.git.get_author_email(commitid, abspath),`
		`+ user=user,`
		`ticketfolder=pagure.APP.config['TICKETS_FOLDER'],`
		`)`
		`elif relation.isa == 'pull-request':`
		`@@ -128,7 +134,7 @@`
		`filename=None,`
		`row=None,`
		`comment=comment,`
		`- user=pagure.lib.git.get_author_email(commitid, abspath),`
		`+ user=user,`
		`requestfolder=pagure.APP.config['REQUESTS_FOLDER'],`
		`)`
		`pagure.SESSION.commit()`
		`@@ -144,14 +150,14 @@`
		`pagure.SESSION,`
		`relation,`
		`ticketfolder=pagure.APP.config['TICKETS_FOLDER'],`
		`- user=pagure.lib.git.get_author_email(commitid, abspath),`
		`+ user=user,`
		`status='Closed', close_status='Fixed')`
		`elif relation.isa == 'pull-request':`
		`pagure.lib.close_pull_request(`
		`pagure.SESSION,`
		`relation,`
		`requestfolder=pagure.APP.config['REQUESTS_FOLDER'],`
		`- user=pagure.lib.git.get_author_email(commitid, abspath),`
		`+ user=user,`
		`merged=True)`
		`pagure.SESSION.commit()`
		`except pagure.exceptions.PagureException as err:`
		`@@ -197,6 +203,7 @@`
		`pagure.lib.git.get_revs_between(oldrev, newrev, abspath, refname))`

		`if pagure.APP.config.get('HOOK_DEBUG', False):`
		`+ print('ns :', pagure.lib.git.get_repo_namespace(abspath))`
		`print('repo:', pagure.lib.git.get_repo_name(abspath))`
		`print('user:', pagure.lib.git.get_username(abspath))`

pingou commented 6 years ago

This was a small annoyance we had, if you were amending a commit from
someone else, or just rewording the commit message (for example to add
the Merges... line) and pushing to the main repo, pagure would show
the action has being done by the author of the commit rather than the
person who pushed that commit.

This commit fixes this by relying on the environment variable set by
gitolite if it is present, else going back to the author of the commit
if we cannot know who pushed.

Fixes https://pagure.io/pagure/issue/2597

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

1 new commit added

Add a small debugging statement

6 years ago

bowlofeggs commented on line 23 of pagure/hooks/files/pagure_hook.py 6 years ago

I wonder whether this would make a good function since it's used twice in this patch.

bowlofeggs commented 6 years ago

I recommend tests, but LGTM otherwise.

pingou commented on line 23 of pagure/hooks/files/pagure_hook.py 6 years ago

For the moment it's pretty straight forward so I'll keep this here but if it's needed in more places it may make sense to move it to a function indeed.

pingou commented 6 years ago

Hooks themselves don't really have tests but the individual functions they use are tested.

Thanks for the review! :)

Pull-Request has been merged by pingou

6 years ago