Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1052803002:
Add script for landing CLs on behalf of external authors. (Closed)
|
Created:
5 years, 8 months ago by nyquist Modified:
5 years, 8 months ago Base URL:
https://github.com/chromium/dom-distiller.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionAdd script for landing CLs on behalf of external authors.
Currently there is no easy way for external authors to directly contribute
to the DOM Distiller repository.
This CL adds a helpful script for members of the DOM Distiller project to
land CLs on behalf of external authors.
BUG=
R=cjhopman@chromium.org, wychen@chromium.org
Committed: 772dbda5236064b67a31d65cd9ebad7f233485fc
Patch Set 1 #Patch Set 2 : Fix color-issue #Patch Set 3 : Added support for printing the description #
Total comments: 12
Messages
Total messages: 15 (2 generated)
nyquist@chromium.org changed reviewers: + cjhopman@chromium.org
cjhopman: PTAL Example commit from 'git log --format=fuller': commit 7eb6a1be7c1886ea8681c5baa111ef7354fc37a9 Author: alex <alex@igalia.com> AuthorDate: Wed Apr 1 15:16:01 2015 -0700 Commit: Tommy Nyquist <nyquist@chromium.org> CommitDate: Wed Apr 1 15:16:01 2015 -0700 The location of the chrome distillation source changed. After https://codereview.chromium.org/987793002 the place where the distillation script is copied has changed. BUG= R=nyquist@chromium.org Review URL: https://codereview.chromium.org/1052613002 Patch from alex <alex@igalia.com>. Commit on GitHub: https://github.com/chromium/dom-distiller/commit/7eb6a1be7c1886ea8681c5baa111...
cjhopman: Fixed color issue. PTAL.
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:33: GIT_EDITOR=cat git cl description | \ This sadly edits the CL description; so it should go away.
wychen@chromium.org changed reviewers: + wychen@chromium.org
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:33: GIT_EDITOR=cat git cl description | \ On 2015/04/02 19:15:30, nyquist wrote: > This sadly edits the CL description; so it should go away. I'll fix this in git_cl.py soon. https://codereview.chromium.org/1058793004/
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:33: GIT_EDITOR=cat git cl description | \ On 2015/04/02 22:36:06, Wei-Yin Chen wrote: > On 2015/04/02 19:15:30, nyquist wrote: > > This sadly edits the CL description; so it should go away. > > I'll fix this in git_cl.py soon. > https://codereview.chromium.org/1058793004/ That's fantastic! Thanks! I guess this is ready for review then.
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:11: get_author_from_first_comment() { We should probably have git cl expose the actual author at some point. https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:36: echo "######" This should probably have some sort of confirm dialog and a way to reject it if its the wrong patch. https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:55: git cl land -c "${author_name} <${author_email}>" does this run presubmits?
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:11: get_author_from_first_comment() { On 2015/04/06 18:28:52, cjhopman wrote: > We should probably have git cl expose the actual author at some point. I have a CL here under review. https://codereview.chromium.org/1065803002/ https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:33: GIT_EDITOR=cat git cl description | \ On 2015/04/03 01:23:31, nyquist (OOO APR 3-7) wrote: > On 2015/04/02 22:36:06, Wei-Yin Chen wrote: > > On 2015/04/02 19:15:30, nyquist wrote: > > > This sadly edits the CL description; so it should go away. > > > > I'll fix this in git_cl.py soon. > > https://codereview.chromium.org/1058793004/ > > That's fantastic! Thanks! > > I guess this is ready for review then. The fix on git_cl.py was committed, so I guess this part is good as is. https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:36: echo "######" On 2015/04/06 18:28:52, cjhopman wrote: > This should probably have some sort of confirm dialog and a way to reject it if > its the wrong patch. If Ctrl-c counts as a way to reject, we already have it. The clean up (something like "git branch -D land-issue-${issue_number}") is not there though. https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:55: git cl land -c "${author_name} <${author_email}>" On 2015/04/06 18:28:52, cjhopman wrote: > does this run presubmits? Adding "ant clean && ant test" before this line?
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:55: git cl land -c "${author_name} <${author_email}>" On 2015/04/07 03:15:40, wychen wrote: > On 2015/04/06 18:28:52, cjhopman wrote: > > does this run presubmits? > > Adding "ant clean && ant test" before this line? Tests aren't the same as presubmits. I believe that right now, presubmits only check that someone actually lgtmed the change. Running tests might also be something we want to do. I don't actually know if we want to do either of those, I just want us to intentionally either run them or not.
https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... File land-external-contributor-cl.sh (right): https://codereview.chromium.org/1052803002/diff/40001/land-external-contribut... land-external-contributor-cl.sh:55: git cl land -c "${author_name} <${author_email}>" On 2015/04/07 16:59:17, cjhopman wrote: > On 2015/04/07 03:15:40, wychen wrote: > > On 2015/04/06 18:28:52, cjhopman wrote: > > > does this run presubmits? > > > > Adding "ant clean && ant test" before this line? > > Tests aren't the same as presubmits. I believe that right now, presubmits only > check that someone actually lgtmed the change. Running tests might also be > something we want to do. > > I don't actually know if we want to do either of those, I just want us to > intentionally either run them or not. We are intentionally not running tests here. We probably could/should though. Adding a mental note about that. This is just an ugly wrapper around `git cl land`. Sadly git cl presubmit does not have a legit exit code, so it doesn't really help us very much.
lgtm
lgtm
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 772dbda5236064b67a31d65cd9ebad7f233485fc (presubmit successful). |
