Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1891793004:
Roll chromite for GN support in Simple Chrome (Closed)
DescriptionRoll chromite for GN support in Simple Chrome
This whitelists chromite third party libraries with non-conforming
lisences.
BUG=561142, 602151, 602830
Committed: https://crrev.com/0ba29de06ec21c07e3f62f5e40a19878e8b28815
Cr-Commit-Position: refs/heads/master@{#387726}
Patch Set 1 #Patch Set 2 : Also roll pyelftools #Patch Set 3 : Add crbug issues for missing lisences. #
Total comments: 10
Patch Set 4 : Add upstream bugs #Patch Set 5 : Also add upstream bugs to other instances #Messages
Total messages: 32 (12 generated)
stevenjb@chromium.org changed reviewers: + hashimoto@chromium.org, phajdan.jr@chromium.org, vapier@chromium.org
See http://crbug.com/602151 for licencing discussion.
you most likely need/should roll pyelftools in parallel -- see chromium:602830
stevenjb@chromium.org changed reviewers: + achuith@google.com
+achuith@
On 2016/04/14 22:53:25, vapier wrote: > you most likely need/should roll pyelftools in parallel -- see chromium:602830 ugh. Looks like we pull pyelftools into src/third_party (instead of src/third_party/chromite/third_party) because it is also in skia's DEPS? (It's not clear that skia actually needs it though...). I'll add a comment to that effect. Any reason not to roll these together? If not I'll update pyelftools also. It seems to work locally.
Description was changed from ========== Roll chromite for GN support in Simple Chrome This whitelists chromite third party libraries with non-conforming lisences. BUG=561142,602151 ========== to ========== Roll chromite for GN support in Simple Chrome This whitelists chromite third party libraries with non-conforming lisences. BUG=561142,602151,602830 ==========
The CQ bit was checked by stevenjb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1891793004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1891793004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Could you just file *upstream* bug reports for missing license headers in the affected directories? Please check whether the headers are missing, or we fail to recognize them.
I'm not actually familiar with any of these libraries and with the exception of httplib2 it is not obvious where they come from. I filed and assigned crbug issues and included them in the comments.
Thanks. Could you please file *upstream* bugs though and make sure the links we put in checklicenses.py point to *upstream* bugs? All processes for importing 3rd party code I'm aware of involve clearly identifying the source where it came from. In exceptional situations (e.g. upstream really doesn't have a bug tracker/website or something like that), just let me know - it won't block this patch. I'd just like to ask for reasonable effort to notify upstream about missing license headers.
The CQ bit was checked by achuith@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1891793004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1891793004/40001
lgtm
I'm trying to move things forward on the GN migration, but I'm also juggling a lot of other things right now. There seemed to be some existing precedent for exceptions with only crbugs filed when the code is not actually shipped. If we really don't want to commit this as is, I will see how full phobbs@'s plate is or see if I can recruit someone more familiar with chromite to file the upstream bugs. On Fri, Apr 15, 2016 at 11:02 AM, <phajdan.jr@chromium.org> wrote: > Thanks. Could you please file *upstream* bugs though and make sure the > links we > put in checklicenses.py point to *upstream* bugs? > > All processes for importing 3rd party code I'm aware of involve clearly > identifying the source where it came from. > > In exceptional situations (e.g. upstream really doesn't have a bug > tracker/website or something like that), just let me know - it won't block > this > patch. > > I'd just like to ask for reasonable effort to notify upstream about missing > license headers. > > https://codereview.chromium.org/1891793004/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thestig@chromium.org changed reviewers: + thestig@chromium.org
https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... File tools/checklicenses/checklicenses.py (right): https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:187: # http://crbug.com/603946 https://github.com/google/oauth2client/issues/331 ? https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:205: # http://crbug.com/603939 https://github.com/jcgregorio/httplib2/issues/307 https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:524: # http://crbug.com/334668 Ditto, https://github.com/jcgregorio/httplib2/issues/307
It looks like you just need to file one more upstream bug on GitHub with a Google project. Can we just do that and then everyone is happy? https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... File tools/checklicenses/checklicenses.py (right): https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:193: # http://crbug.com/603946 I believe upstream is https://github.com/google/google-api-python-client https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:217: # http://crbug.com/603944 Already filed at https://github.com/kennethreitz/requests/issues/1610
Thanks Lei! PTAL https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... File tools/checklicenses/checklicenses.py (right): https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:187: # http://crbug.com/603946 On 2016/04/15 20:51:31, Lei Zhang wrote: > https://github.com/google/oauth2client/issues/331 ? Thanks! https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:193: # http://crbug.com/603946 On 2016/04/15 20:56:58, Lei Zhang wrote: > I believe upstream is https://github.com/google/google-api-python-client Done. https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:205: # http://crbug.com/603939 On 2016/04/15 20:51:31, Lei Zhang wrote: > https://github.com/jcgregorio/httplib2/issues/307 Acknowledged. https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:217: # http://crbug.com/603944 On 2016/04/15 20:56:58, Lei Zhang wrote: > Already filed at https://github.com/kennethreitz/requests/issues/1610 Acknowledged. https://codereview.chromium.org/1891793004/diff/40001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:524: # http://crbug.com/334668 On 2016/04/15 20:51:31, Lei Zhang wrote: > Ditto, https://github.com/jcgregorio/httplib2/issues/307 Done.
lgtm
The CQ bit was checked by stevenjb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from achuith@chromium.org Link to the patchset: https://codereview.chromium.org/1891793004/#ps80001 (title: "Also add upstream bugs to other instances")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1891793004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1891793004/80001
Message was sent while issue was closed.
Description was changed from ========== Roll chromite for GN support in Simple Chrome This whitelists chromite third party libraries with non-conforming lisences. BUG=561142,602151,602830 ========== to ========== Roll chromite for GN support in Simple Chrome This whitelists chromite third party libraries with non-conforming lisences. BUG=561142,602151,602830 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Roll chromite for GN support in Simple Chrome This whitelists chromite third party libraries with non-conforming lisences. BUG=561142,602151,602830 ========== to ========== Roll chromite for GN support in Simple Chrome This whitelists chromite third party libraries with non-conforming lisences. BUG=561142,602151,602830 Committed: https://crrev.com/0ba29de06ec21c07e3f62f5e40a19878e8b28815 Cr-Commit-Position: refs/heads/master@{#387726} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/0ba29de06ec21c07e3f62f5e40a19878e8b28815 Cr-Commit-Position: refs/heads/master@{#387726} |
