|
|
Chromium Code Reviews|
Created:
4 years, 1 month ago by Zhiqiang Zhang (Slow) Modified:
4 years, 1 month ago CC:
agrieve+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, jam, Jinsuk Kim Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove DEPS restriction content_public/ to content/ (Java)
The DEPS rules for Java content_public/ is wrong. It should be able to
import classes from content/. This CL removes the restriction.
BUG=658678, 335690
Committed: https://crrev.com/bfe32da7ffd850ddfdd58f6c775f0dc1a92eed78
Cr-Commit-Position: refs/heads/master@{#428684}
Patch Set 1 #
Messages
Total messages: 20 (11 generated)
Description was changed from ========== Remove DEPS restriction to content/ from content_public/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=NONE ========== to ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=NONE ==========
zqzhang@chromium.org changed reviewers: + boliu@chromium.org
PTAL
boliu@chromium.org changed reviewers: + aelias@chromium.org, dtrainor@chromium.org, tedchoc@chromium.org, yfriedman@chromium.org
add all the content/public/android owners. but most are not here, so I'm going to make a decision here this DEPS rule makes no sense, and it's exactly the opposite of this DEPS rule for content/public/browser: https://cs.chromium.org/chromium/src/content/public/browser/DEPS?rcl=0&l=3 while it's nice if content_public only had interfaces which shouldn't depend on implementation, static methods is still a thing, and especially in java, can't just be magically implemented somewhere else specifically it's to address implement this static method: https://codereview.chromium.org/2444833004/diff/100001/content/public/android...
lgtm
Seems OK given that it's the opposite of the other restriction. Could you link the motivating bug number? Is it http://crbug.com/658678 ?
also link crbug.com/335690 maybe?
Description was changed from ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=NONE ========== to ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=658678,335690 ==========
On 2016/10/29 00:23:00, boliu wrote: > also link crbug.com/335690 maybe? > Seems OK given that it's the opposite of the other restriction. Could you link > the motivating bug number? Is it http://crbug.com/658678 ? Done for the links
The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by zqzhang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=658678,335690 ========== to ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=658678,335690 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=658678,335690 ========== to ========== Remove DEPS restriction content_public/ to content/ (Java) The DEPS rules for Java content_public/ is wrong. It should be able to import classes from content/. This CL removes the restriction. BUG=658678,335690 Committed: https://crrev.com/bfe32da7ffd850ddfdd58f6c775f0dc1a92eed78 Cr-Commit-Position: refs/heads/master@{#428684} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/bfe32da7ffd850ddfdd58f6c775f0dc1a92eed78 Cr-Commit-Position: refs/heads/master@{#428684} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
