Use chromite sync_chrome script to sync Chrome in Chrome ebu... [chromiumos/overlays/chromiumos-overlay : master]

42 views
Skip to first unread message

David James (Code Review)

unread,
Nov 13, 2012, 8:49:32 PM11/13/12
to
David James has uploaded a new change for review.

Change subject: Use chromite sync_chrome script to sync Chrome in Chrome ebuild.
......................................................................

Use chromite sync_chrome script to sync Chrome in Chrome ebuild.

BUG=chromium-os:36324
CQ-DEPEND=37923
TEST=Remote trybot run.

Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
---
M chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
1 file changed, 10 insertions(+), 182 deletions(-)


git pull ssh://gerrit.chromium.org:29418/chromiumos/overlays/chromiumos-overlay refs/changes/65/37965/1
--
To view, visit https://gerrit.chromium.org/gerrit/37965
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/overlays/chromiumos-overlay
Gerrit-Branch: master
Gerrit-Owner: David James <david...@chromium.org>

Peter Mayo (Code Review)

unread,
Nov 14, 2012, 1:27:44 AM11/14/12
to David James, Gerrit, Chris Sosa, Ryan Cui
Peter Mayo has posted comments on this change.

Change subject: Use chromite sync_chrome script to sync Chrome in Chrome ebuild.
......................................................................


Patch Set 1: Looks good to me, approved

(3 inline comments)

....................................................
File chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
Line 363: flags+=( "${CHROME_DISTDIR}" )
Consider "elog" the subcommand to subdivide problem solving and interpretation.

Line 364: "${CROS_WORKON_SRCROOT}"/chromite/bin/sync_chrome "${flags[@]}" || die
Message for the "die"?

....................................................
Commit Message
Line 11: TEST=Remote trybot run.
There are lots of cases to validate - hopefully more than one try run.

--
To view, visit https://gerrit.chromium.org/gerrit/37965
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/overlays/chromiumos-overlay
Gerrit-Branch: master
Gerrit-Owner: David James <david...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>
Gerrit-Reviewer: Peter Mayo <pete...@chromium.org>
Gerrit-Reviewer: Ryan Cui <rc...@chromium.org>

Chris Sosa (Code Review)

unread,
Nov 14, 2012, 7:54:08 AM11/14/12
to David James, Gerrit, Ryan Cui, Peter Mayo
Chris Sosa has posted comments on this change.

Change subject: Use chromite sync_chrome script to sync Chrome in Chrome ebuild.
......................................................................


Patch Set 1: Looks good to me, approved

(1 inline comment)

....................................................
File chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
Line 355: local flags=( --reset --gclient="${EGCLIENT}" )
might be worth a comment on why --reset is always used.

--
To view, visit https://gerrit.chromium.org/gerrit/37965
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/overlays/chromiumos-overlay
Gerrit-Branch: master
Gerrit-Owner: David James <david...@chromium.org>

David James (Code Review)

unread,
Nov 14, 2012, 11:54:02 PM11/14/12
to Gerrit, Chris Sosa, Ryan Cui, Peter Mayo
David James has posted comments on this change.

Change subject: Use chromite sync_chrome script to sync Chrome in Chrome ebuild.
......................................................................


Patch Set 1: (4 inline comments)

....................................................
File chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild
Line 355: local flags=( --reset --gclient="${EGCLIENT}" )
Done

Line 363: flags+=( "${CHROME_DISTDIR}" )
Done

Line 364: "${CROS_WORKON_SRCROOT}"/chromite/bin/sync_chrome "${flags[@]}" || die
The convention for ebuilds is just to do '|| die' since ebuild already tells you what command failed.

....................................................
Commit Message
Line 11: TEST=Remote trybot run.
Yup, ran lots of trybot runs :) Updated the description.

--
To view, visit https://gerrit.chromium.org/gerrit/37965
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/overlays/chromiumos-overlay
Gerrit-Branch: master
Gerrit-Owner: David James <david...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: David James <david...@chromium.org>

David James (Code Review)

unread,
Nov 15, 2012, 11:34:33 AM11/15/12
to Gerrit, Chris Sosa, Ryan Cui, Peter Mayo
David James has posted comments on this change.

Change subject: Use chromite sync_chrome script to sync Chrome in Chrome ebuild.
......................................................................


Patch Set 2: Verified; Looks good to me, approved; Ready

All trybot runs are passing. Inheriting LGTM.

--
To view, visit https://gerrit.chromium.org/gerrit/37965
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/overlays/chromiumos-overlay
Gerrit-Branch: master
Gerrit-Owner: David James <david...@chromium.org>

Peter Mayo (Code Review)

unread,
Nov 15, 2012, 11:42:45 AM11/15/12
to David James, Gerrit, Chris Sosa, Ryan Cui
Peter Mayo has posted comments on this change.

Change subject: Use chromite sync_chrome script to sync Chrome in Chrome ebuild.
......................................................................


Patch Set 2: Looks good to me, approved

--
To view, visit https://gerrit.chromium.org/gerrit/37965
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70628e8c930f3df9cb1fced12f26a3aeb44a1f5e
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/overlays/chromiumos-overlay
Gerrit-Branch: master
Gerrit-Owner: David James <david...@chromium.org>
Reply all
Reply to author
Forward
0 new messages