New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 593533 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

More clear error needed "Missing LGTM from OWNERS of dependencies added to DEPS"

Project Member Reported by loyso@chromium.org, Mar 10 2016

Issue description

I can't land this CL:
https://codereview.chromium.org/1747783002/
beacause of chrome/browser/android/DEPS modification
even after two LGTMS from dtrainor@
who is an OWNER in chrome/browser/android/
 

Comment 1 by loyso@chromium.org, Mar 11 2016

Labels: -Pri-1 -Type-Bug Pri-2 Type-Feature
Summary: More clear error needed "Missing LGTM from OWNERS of dependencies added to DEPS" (was: "Missing LGTM from OWNERS of dependencies added to DEPS" after LGTM from OWNER)
** Presubmit ERRORS **
Missing LGTM from OWNERS of dependencies added to DEPS:
    '+cc/layers/layer_settings.h',

"You need someone from CC because you're adding that as a dependency.  They have to approve you depending on them."

It doesn't explain that you need a CC OWNER LGTM.

More clear message needed. And suggested reviewers.
Labels: -Infra Infra-CommitQueue
Status: Untriaged (was: Available)
As clarified by loyso, "You need someone from CC because you're adding that as a dependency.  They have to approve you depending on them." is a message from the chat, not a suggested text.

IMHO, "Missing LGTM from OWNERS of dependencies added to DEPS" is already clear, but perhaps we can make it even more clear by adding

Please ask OWNERs of added dependencies to review this CL.

in the end?
After some further offline discussion with loyso, we think we should change this to:

** Presubmit ERRORS **
You need LGTM from owners of depends-on paths in DEPS that were modified in this CL:
    '+cc/layers/layer_settings.h',
Owner: phajdan.jr@chromium.org
Status: Assigned (was: Untriaged)

Comment 6 by aga...@chromium.org, Apr 26 2016

Components: Infra>CQ
Labels: -Infra-CommitQueue
Status: Started (was: Assigned)
Uploaded https://codereview.chromium.org/2120283002 .
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec17f88453662349b4ab73a51cf74bd416eca2da

commit ec17f88453662349b4ab73a51cf74bd416eca2da
Author: Paweł Hajdan, Jr <phajdan.jr@chromium.org>
Date: Mon Jul 04 14:16:15 2016

Improve handling of DEPS OWNERS PRESUBMIT check

BUG= 593533 

Review URL: https://codereview.chromium.org/2120283002 .

Cr-Commit-Position: refs/heads/master@{#403680}

[modify] https://crrev.com/ec17f88453662349b4ab73a51cf74bd416eca2da/PRESUBMIT.py

Status: Fixed (was: Started)

Sign in to add a comment