Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2539)

Issue 2424093003: Include the AppData file for Chrome and Chromium (Closed)

Created:
2 years, 12 months ago by tpopela
Modified:
2 years, 2 months ago
CC:
chromium-reviews, grt+watch_chromium.org, pennymac+watch_chromium.org, wfh+watch_chromium.org, phajdan
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Include the AppData file for Chrome and Chromium Add the two AppData files, one for Chromium and one for Chrome. The Chrome one will be filled with the information from the common/google-chrome/google-chrome.info file and should reflect the current channel (stable, beta, devel). On the other side the Chromium one should be a general one and it will be reused by distributions that are packaging the Chromium themselves. For AppData specification please see: https://people.freedesktop.org/~hughsient/appdata/ BUG=654190 Review-Url: https://codereview.chromium.org/2424093003 Cr-Original-Original-Commit-Position: refs/heads/master@{#455457} Committed: https://chromium.googlesource.com/chromium/src/+/d6807b9a7da70dae9191b93d4ec6157187515078 Review-Url: https://codereview.chromium.org/2424093003 Cr-Original-Commit-Position: refs/heads/master@{#455731} Committed: https://chromium.googlesource.com/chromium/src/+/caa17f475b1ccf16ce0d4f89902663f4462396a8 Review-Url: https://codereview.chromium.org/2424093003 Cr-Commit-Position: refs/heads/master@{#458406} Committed: https://chromium.googlesource.com/chromium/src/+/dd765c0d5ff5089329188018ac826e0728cad15e

Patch Set 1 #

Total comments: 1

Patch Set 2 : Change Chromium appdata description #

Patch Set 3 : Add screenshot, feedback link and fix some of appstream-util validation warnings #

Total comments: 1

Patch Set 4 : Use the right directory with the AppData files while installing #

Patch Set 5 : Fix the build and introduce the AppData template even for Chromium #

Total comments: 1

Patch Set 6 : Remove the Chromium AppData template to avoid duplicity #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -2 lines) Patch
M chrome/installer/linux/BUILD.gn View 1 5 1 chunk +8 lines, -2 lines 0 comments Download
A chrome/installer/linux/common/chromium-browser/chromium-browser.appdata.xml View 1 2 1 chunk +36 lines, -0 lines 0 comments Download
A chrome/installer/linux/common/google-chrome/google-chrome.appdata.xml.template View 1 2 1 chunk +26 lines, -0 lines 0 comments Download
M chrome/installer/linux/common/installer.include View 1 2 3 4 5 2 chunks +11 lines, -0 lines 0 comments Download
M chrome/installer/linux/rpm/chrome.spec.template View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 56 (26 generated)
Paweł Hajdan Jr.
I'm happy to get back to you with some results of internal review. I'm still ...
2 years, 7 months ago (2017-03-01 16:23:36 UTC) #4
tpopela
Update the description based on Pawel's feedback. Also there is a new pre-submit warning: Found ...
2 years, 7 months ago (2017-03-02 11:14:26 UTC) #5
Paweł Hajdan Jr.
Please use https://support.google.com/chrome/?p=feedback (it'll get linked to the proper destination soon). I'll also point you ...
2 years, 7 months ago (2017-03-07 18:18:58 UTC) #6
tpopela
On 2017/03/07 18:18:58, Paweł Hajdan Jr. wrote: > Please use https://support.google.com/chrome/?p=feedback (it'll get linked to ...
2 years, 7 months ago (2017-03-08 07:35:17 UTC) #7
Paweł Hajdan Jr.
Sounds good. Please use https://www.gstatic.com/chrome/appstream/chrome-1.png for the screenshot. Chrome and Chromium look the same, I ...
2 years, 7 months ago (2017-03-08 12:37:08 UTC) #8
tpopela
Add screenshot, feedback link and fix some of appstream-util validation warnings. There are still some ...
2 years, 7 months ago (2017-03-08 13:27:23 UTC) #9
Paweł Hajdan Jr.
LGTM
2 years, 7 months ago (2017-03-08 13:31:34 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2424093003/40001
2 years, 7 months ago (2017-03-08 15:29:32 UTC) #16
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/d6807b9a7da70dae9191b93d4ec6157187515078
2 years, 7 months ago (2017-03-08 15:34:26 UTC) #19
xhwang
This is breaking Google Chrome Linux bots. https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux%20x64/builds/16701
2 years, 7 months ago (2017-03-08 17:33:52 UTC) #21
xhwang
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2734783009/ by xhwang@chromium.org. ...
2 years, 7 months ago (2017-03-08 17:35:12 UTC) #22
xhwang
On 2017/03/08 17:35:12, xhwang_slow wrote: > A revert of this CL (patchset #3 id:40001) has ...
2 years, 7 months ago (2017-03-08 21:52:09 UTC) #23
Michael Moss
On 2017/03/08 21:52:09, xhwang_slow wrote: > On 2017/03/08 17:35:12, xhwang_slow wrote: > > A revert ...
2 years, 7 months ago (2017-03-08 21:58:53 UTC) #24
xhwang
On 2017/03/08 21:58:53, Michael Moss wrote: > On 2017/03/08 21:52:09, xhwang_slow wrote: > > On ...
2 years, 7 months ago (2017-03-08 22:09:03 UTC) #25
tpopela
In chrome/installer/linux/BUILD.gn the files are copied to ${BUILDDIR}/installer/common/ and not to ${BUILDDIR}/installer/common/google-chrome/ or ${BUILDDIR}/installer/common/chromium/.
2 years, 7 months ago (2017-03-09 09:35:22 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2424093003/60001
2 years, 7 months ago (2017-03-09 11:37:49 UTC) #34
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/caa17f475b1ccf16ce0d4f89902663f4462396a8
2 years, 7 months ago (2017-03-09 13:56:03 UTC) #37
fserb
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2738043003/ by fserb@chromium.org. ...
2 years, 7 months ago (2017-03-09 15:54:13 UTC) #38
tpopela
Previously the builds failed because we were not creating the /usr/share/appdata directory in the build ...
2 years, 7 months ago (2017-03-17 14:46:40 UTC) #39
Paweł Hajdan Jr.
https://codereview.chromium.org/2424093003/diff/80001/chrome/installer/linux/common/chromium-browser/chromium-browser.appdata.xml File chrome/installer/linux/common/chromium-browser/chromium-browser.appdata.xml (right): https://codereview.chromium.org/2424093003/diff/80001/chrome/installer/linux/common/chromium-browser/chromium-browser.appdata.xml#newcode1 chrome/installer/linux/common/chromium-browser/chromium-browser.appdata.xml:1: <!-- Copyright 2017 The Chromium Authors --> Is the ...
2 years, 7 months ago (2017-03-20 10:47:24 UTC) #40
tpopela
On 2017/03/20 10:47:24, Paweł Hajdan Jr. wrote: > Is the non-template file used? No, that ...
2 years, 7 months ago (2017-03-20 10:53:21 UTC) #42
Paweł Hajdan Jr.
On 2017/03/20 10:53:21, tpopela wrote: > On 2017/03/20 10:47:24, Paweł Hajdan Jr. wrote: > > ...
2 years, 6 months ago (2017-03-21 13:04:06 UTC) #44
tpopela
On 2017/03/21 13:04:06, Paweł Hajdan Jr. wrote: > Can we somehow get rid of the ...
2 years, 6 months ago (2017-03-21 13:08:23 UTC) #45
Paweł Hajdan Jr.
On 2017/03/21 13:08:23, tpopela wrote: > On 2017/03/21 13:04:06, Paweł Hajdan Jr. wrote: > > ...
2 years, 6 months ago (2017-03-21 13:17:37 UTC) #46
tpopela
Removed the Chromium AppData template to avoid duplicity between it and the regular AppData file.
2 years, 6 months ago (2017-03-21 13:34:15 UTC) #47
Paweł Hajdan Jr.
LGTM, thanks!
2 years, 6 months ago (2017-03-21 13:48:10 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2424093003/100001
2 years, 6 months ago (2017-03-21 13:49:00 UTC) #50
commit-bot: I haz the power
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/dd765c0d5ff5089329188018ac826e0728cad15e
2 years, 6 months ago (2017-03-21 14:45:04 UTC) #53
Lei Zhang
https://codereview.chromium.org/2424093003/diff/40001/chrome/installer/linux/common/installer.include File chrome/installer/linux/common/installer.include (right): https://codereview.chromium.org/2424093003/diff/40001/chrome/installer/linux/common/installer.include#newcode231 chrome/installer/linux/common/installer.include:231: if [ ${PACKAGE:0:6} = google ]; then Curious, why ...
2 years, 2 months ago (2017-07-25 22:13:59 UTC) #55
Lei Zhang
2 years, 2 months ago (2017-07-25 22:45:24 UTC) #56
Message was sent while issue was closed.
On 2017/07/25 22:13:59, Lei Zhang wrote:
>
https://codereview.chromium.org/2424093003/diff/40001/chrome/installer/linux/...
> File chrome/installer/linux/common/installer.include (right):
> 
>
https://codereview.chromium.org/2424093003/diff/40001/chrome/installer/linux/...
> chrome/installer/linux/common/installer.include:231: if [ ${PACKAGE:0:6} =
> google ]; then
> Curious, why not just check ${PACKAGE} ? The only possible values are
> google-chrome and chromium-browser.

Uploaded https://chromium-review.googlesource.com/c/585454/

Powered by Google App Engine
This is Rietveld 408576698