Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(718)

Issue 11868043: Add ability to review commit messages.

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by jparent1
Modified:
1 month ago
CC:
codereview-list_googlegroups.com
Visibility:
Public.

Description

Add ability to review commit messages. 1. When you upload a new issue, creates a magic COMMIT_MSG file that is part of the patchset, with the contents of the issue description. This file is not included in the raw patchset, since it isn't a real file that you'd want to download, but shows up in the UI. This allows you to add comments to the commit message, view diffs between versions, etc. 2. If you edit the description from the Reitveld UI, a new patchset is created, identical to the last one, except with a new COMMIT_MSG, to keep it in sync with the one in the issue description. https://code.google.com/p/rietveld/issues/detail?id=457

Patch Set 1 #

Total comments: 8

Patch Set 2 : Addressing code review feedback #

Total comments: 12

Patch Set 3 : Address stylistic code review feedback. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -0 lines) Patch
M codereview/views.py View 1 2 7 chunks +75 lines, -0 lines 0 comments Download
M upload.py View 1 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 15
jparent1
Demo server is running, with demo issue, at https://codereview-julie-test.appspot.com/9001/ You can see the new special ...
11 years ago (2013-07-25 20:58:53 UTC) #1
jparent1
https://codereview.appspot.com/11868043/diff/1/app.yaml File app.yaml (right): https://codereview.appspot.com/11868043/diff/1/app.yaml#newcode1 app.yaml:1: application: codereview-julie-test This obviously shouldn't be part of this ...
11 years ago (2013-07-26 00:50:56 UTC) #2
tfarina1
https://codereview.appspot.com/11868043/diff/1/codereview/views.py File codereview/views.py (right): https://codereview.appspot.com/11868043/diff/1/codereview/views.py#newcode1838 codereview/views.py:1838: filename = "COMMIT_MSG" Can you rename this to "Commit ...
11 years ago (2013-07-26 00:55:55 UTC) #3
iannucci
https://codereview.appspot.com/11868043/diff/1/app.yaml File app.yaml (right): https://codereview.appspot.com/11868043/diff/1/app.yaml#newcode1 app.yaml:1: application: codereview-julie-test On 2013/07/26 00:50:56, jparent1 wrote: > This ...
11 years ago (2013-07-26 00:59:27 UTC) #4
Andi
On 2013/07/26 00:59:27, iannucci wrote: > https://codereview.appspot.com/11868043/diff/1/app.yaml > File app.yaml (right): > > https://codereview.appspot.com/11868043/diff/1/app.yaml#newcode1 > ...
10 years, 12 months ago (2013-07-29 08:30:50 UTC) #5
Andi
https://codereview.appspot.com/11868043/diff/1/codereview/views.py File codereview/views.py (right): https://codereview.appspot.com/11868043/diff/1/codereview/views.py#newcode1838 codereview/views.py:1838: filename = "COMMIT_MSG" I'd also would like to see ...
10 years, 12 months ago (2013-07-29 08:38:34 UTC) #6
jparent1
Addressed all feedback. Special commit file is no longer part of tarball, is now named ...
10 years, 11 months ago (2013-08-14 20:16:51 UTC) #7
jparent1
Any other concerns? Thanks!
10 years, 11 months ago (2013-08-19 17:58:09 UTC) #8
iannucci
looks pretty good to me, but I'd like someone to OK the transactionality of the ...
10 years, 11 months ago (2013-08-19 21:56:47 UTC) #9
M-A
https://codereview.appspot.com/11868043/diff/12001/codereview/views.py File codereview/views.py (right): https://codereview.appspot.com/11868043/diff/12001/codereview/views.py#newcode1837 codereview/views.py:1837: 2 lines between file level symbols (and line 1854) ...
10 years, 11 months ago (2013-08-20 01:24:28 UTC) #10
Andi
https://codereview.appspot.com/11868043/diff/12001/codereview/views.py File codereview/views.py (right): https://codereview.appspot.com/11868043/diff/12001/codereview/views.py#newcode1841 codereview/views.py:1841: fromfile="\dev\\null",tofile=COMMIT_MSG) On 2013/08/19 21:56:47, iannucci (OOO through Aug 27) ...
10 years, 11 months ago (2013-08-26 09:39:42 UTC) #11
jparent1
https://codereview.appspot.com/11868043/diff/12001/codereview/views.py File codereview/views.py (right): https://codereview.appspot.com/11868043/diff/12001/codereview/views.py#newcode1837 codereview/views.py:1837: On 2013/08/20 01:24:29, M-A wrote: > 2 lines between ...
10 years, 11 months ago (2013-08-27 22:46:38 UTC) #12
jparent1
On 2013/08/27 22:46:38, jparent1 wrote: > https://codereview.appspot.com/11868043/diff/12001/codereview/views.py > File codereview/views.py (right): > > https://codereview.appspot.com/11868043/diff/12001/codereview/views.py#newcode1837 > ...
10 years, 10 months ago (2013-09-06 22:55:47 UTC) #13
iannucci
On 2013/09/06 22:55:47, jparent1 wrote: > On 2013/08/27 22:46:38, jparent1 wrote: > > https://codereview.appspot.com/11868043/diff/12001/codereview/views.py > ...
10 years, 10 months ago (2013-09-12 01:55:47 UTC) #14
M-A
10 years, 9 months ago (2013-10-16 20:01:44 UTC) #15
It'd be nice if you can complete this CL today, I'm on a committing spree.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b