ios/third_party/firebase lacks a README.chromium |
||||||||||||
Issue description
Unlike other directories under //ios/third_party, there's no README.chromium with licensing information in firebase/.
This causes //tools/licenses.py to fail with:
licenses.LicenseError: missing README.chromium or licenses.py SPECIAL_CASES entry in ios/third_party/firebase
,
Sep 20
Firebase usage can be configured to TRUE or FALSE using ios_enable_firebase_sdk flag in ios/third_party/firebase/firebase.gni. The default is FALSE. When this is FALSE, //tools/licenses.py should not generate the Firebase licenses into chrome://credits. sdefresne: Do you know if there's a way to optionally include certain directories as input into //tools/licenses.py ?
,
Sep 24
Do note that generating about:credits is just one of the things licenses.py can do. "./tools/licenses.py scan" does not need a build directory (and thus |ios_enable_firebase_sdk| is not taken into account). IMO it is easier to just add a README.chromium like the other directories do (see //third_party/README.chromium.template for what fields are required).
,
Oct 19
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/398c144c8d1eb8b6aaccf5daa8597237871026d1 commit 398c144c8d1eb8b6aaccf5daa8597237871026d1 Author: Peter K. Lee <pkl@chromium.org> Date: Fri Oct 19 15:52:34 2018
,
Oct 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16846764f888a65c87328abd02f10ce32991bedb commit 16846764f888a65c87328abd02f10ce32991bedb Author: Peter K. Lee <pkl@chromium.org> Date: Fri Oct 19 16:08:59 2018 Adds README.chromium for chrome://credits page generation Bug: 884663 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I0075911b60df18873c8c64db17d3455d567e9ea6 Reviewed-on: https://chromium-review.googlesource.com/c/1236661 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Justin Cohen <justincohen@chromium.org> Commit-Queue: Peter Lee <pkl@chromium.org> Cr-Commit-Position: refs/heads/master@{#601171} [add] https://crrev.com/16846764f888a65c87328abd02f10ce32991bedb/ios/third_party/firebase/LICENSE [add] https://crrev.com/16846764f888a65c87328abd02f10ce32991bedb/ios/third_party/firebase/README.chromium
,
Oct 19
,
Oct 24
Thanks!
,
Oct 25
Verified in 72.0.3591.0 canary. Settings > Google Chrome > Open Source Licenses opens chrome://credits which now lists Firebase library. Requesting merge to M71 branch for commit in comment 5 (only).
,
Oct 25
The bug is marked as P3 or Feature. It should not be merged as M71 is in beta. Please contact the approriate milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 25
Chrome integrates with Firebase SDK starting with M71. That's third_party code. It is my understanding that acknowledgement in chrome://credits is required. Raising to P2 and re-requesting merge.
,
Oct 25
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 26
,
Oct 31
Approved.
,
Oct 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/335da07eccfb680c6c244ab1a4d1a521d782918a commit 335da07eccfb680c6c244ab1a4d1a521d782918a Author: Peter K. Lee <pkl@chromium.org> Date: Wed Oct 31 23:48:23 2018 Adds README.chromium for chrome://credits page generation Bug: 884663 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I0075911b60df18873c8c64db17d3455d567e9ea6 Reviewed-on: https://chromium-review.googlesource.com/c/1236661 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Justin Cohen <justincohen@chromium.org> Commit-Queue: Peter Lee <pkl@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#601171}(cherry picked from commit 16846764f888a65c87328abd02f10ce32991bedb) Reviewed-on: https://chromium-review.googlesource.com/c/1311113 Reviewed-by: Peter Lee <pkl@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#441} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [add] https://crrev.com/335da07eccfb680c6c244ab1a4d1a521d782918a/ios/third_party/firebase/LICENSE [add] https://crrev.com/335da07eccfb680c6c244ab1a4d1a521d782918a/ios/third_party/firebase/README.chromium
,
Oct 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/335da07eccfb680c6c244ab1a4d1a521d782918a Commit: 335da07eccfb680c6c244ab1a4d1a521d782918a Author: pkl@chromium.org Commiter: pkl@chromium.org Date: 2018-10-31 23:48:23 +0000 UTC Adds README.chromium for chrome://credits page generation Bug: 884663 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I0075911b60df18873c8c64db17d3455d567e9ea6 Reviewed-on: https://chromium-review.googlesource.com/c/1236661 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Justin Cohen <justincohen@chromium.org> Commit-Queue: Peter Lee <pkl@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#601171}(cherry picked from commit 16846764f888a65c87328abd02f10ce32991bedb) Reviewed-on: https://chromium-review.googlesource.com/c/1311113 Reviewed-by: Peter Lee <pkl@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#441} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 7
Verified on 71.0.3578.41 Beta, iPhoneX iOS 11.4.1 https://drive.google.com/file/d/1DJJnb-u0fl9X5O-w7xQ51MKBOCzaRuPd/view Looks good. |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by pkl@chromium.org
, Sep 19