|
|
Created:
6 years, 10 months ago by qyearsley Modified:
6 years, 10 months ago Reviewers:
kjellander_chromium CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/tools/perf.git@master Visibility:
Public. |
DescriptionOmit protocol part in URLs in changelog pages.
Reason for this change:
In the past, build.chromium.org was HTTP-only, and so it couldn't be directly put in an iframe on the perf dashboard which uses HTTPS. Annie Sullivan made a page on the perf dashboard, /revision_proxy to get around this.
Now, build.chromium.org supports HTTPS, and I wanted to switch the changelog viewer links that the dashboard uses to HTTPS. The purpose of this CL is to make the changelog viewer pages work the same with both HTTP and HTTPS.
BUG=
Patch Set 1 #Patch Set 2 : Omit protocol in js/changelog.js as well. #
Messages
Total messages: 23 (0 generated)
Hi Henrik, Could you look at this change for in the perf/dashboard/ui/changelog*.html pages? The reason for this change: In the past, build.chromium.org was HTTP-only, and so it couldn't be directly put in an iframe on the perf dashboard which uses HTTPS. Annie Sullivan made a page on the perf dashboard, /revision_proxy to get around this. Now, build.chromium.org supports HTTPS, and I wanted to witch the changelog viewer links that the dashboard uses to HTTPS. The purpose of this CL is to make the changelog viewer pages work the same with both HTTP and HTTPS. (It's also mentioned in the Google HTML style guide that protocol should be omitted in URLs, in general).
Note: I'm not completely sure whether it's necessary to also change the URLs for src.chromium.org, v8.googlecode.com and webrtc.googlecode.com, but all of these sites support HTTPS as well as HTTP, so this should be fine as far as I know.
On 2014/01/30 00:30:08, qyearsley wrote: > Note: I'm not completely sure whether it's necessary to also change the URLs for > http://src.chromium.org, http://v8.googlecode.com and http://webrtc.googlecode.com, but all of these > sites support HTTPS as well as HTTP, so this should be fine as far as I know. lgtm. Please update the CL description to be the topic. Maybe you can add a few lines from your first publish-message as well, for context?
On 2014/01/31 08:39:51, Henrik Kjellander wrote: > On 2014/01/30 00:30:08, qyearsley wrote: > > Note: I'm not completely sure whether it's necessary to also change the URLs > for > > http://src.chromium.org, http://v8.googlecode.com and > http://webrtc.googlecode.com, but all of these > > sites support HTTPS as well as HTTP, so this should be fine as far as I know. > > lgtm. Please update the CL description to be the topic. Maybe you can add a few > lines from your first publish-message as well, for context? Alright, description updated. What is a publish-message?
On 2014/02/04 01:01:00, qyearsley wrote: > On 2014/01/31 08:39:51, Henrik Kjellander wrote: > > On 2014/01/30 00:30:08, qyearsley wrote: > > > Note: I'm not completely sure whether it's necessary to also change the URLs > > for > > > http://src.chromium.org, http://v8.googlecode.com and > > http://webrtc.googlecode.com, but all of these > > > sites support HTTPS as well as HTTP, so this should be fine as far as I > know. > > > > lgtm. Please update the CL description to be the topic. Maybe you can add a > few > > lines from your first publish-message as well, for context? > > Alright, description updated. What is a publish-message? I meant the text that was sent out in the first e-mail in this issue: https://codereview.chromium.org/148963013/#msg1 It seems the stuff you put in there explains why this change is useful, which makes sense to have in the SVN/Git history. I called it "publish message" since it's what is sent when you click the Publish+Mail comments link :)
On 2014/02/04 07:51:43, Henrik Kjellander wrote: > On 2014/02/04 01:01:00, qyearsley wrote: > > On 2014/01/31 08:39:51, Henrik Kjellander wrote: > > > On 2014/01/30 00:30:08, qyearsley wrote: > > > > Note: I'm not completely sure whether it's necessary to also change the > URLs > > > for > > > > http://src.chromium.org, http://v8.googlecode.com and > > > http://webrtc.googlecode.com, but all of these > > > > sites support HTTPS as well as HTTP, so this should be fine as far as I > > know. > > > > > > lgtm. Please update the CL description to be the topic. Maybe you can add a > > few > > > lines from your first publish-message as well, for context? > > > > Alright, description updated. What is a publish-message? > > I meant the text that was sent out in the first e-mail in this issue: > https://codereview.chromium.org/148963013/#msg1 > It seems the stuff you put in there explains why this change is useful, which > makes sense to have in the SVN/Git history. > I called it "publish message" since it's what is sent when you click the > Publish+Mail comments link :) Ah, right, thanks. Description updated.
The CQ bit was checked by qyearsley@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qyearsley@chromium.org/148963013/20001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qyearsley@chromium.org/148963013/20001
Failed to apply patch for perf/dashboard/ui/changelog.html: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file perf/dashboard/ui/changelog.html Hunk #1 FAILED at 58. 1 out of 1 hunk FAILED -- saving rejects to file perf/dashboard/ui/changelog.html.rej Patch: perf/dashboard/ui/changelog.html Index: dashboard/ui/changelog.html diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html index ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 100644 --- a/perf/dashboard/ui/changelog.html +++ b/perf/dashboard/ui/changelog.html @@ -58,8 +58,8 @@ form { </form> <script> -var svnRootUrl = "http://src.chromium.org/svn/"; -var viewVc = "http://src.chromium.org/viewvc/chrome"; +var svnRootUrl = "//src.chromium.org/svn/"; +var viewVc = "//src.chromium.org/viewvc/chrome"; params = ParseParams(); var svnPath = params["url"];
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qyearsley@chromium.org/148963013/20001
Failed to apply patch for perf/dashboard/ui/changelog.html: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file perf/dashboard/ui/changelog.html Hunk #1 FAILED at 58. 1 out of 1 hunk FAILED -- saving rejects to file perf/dashboard/ui/changelog.html.rej Patch: perf/dashboard/ui/changelog.html Index: dashboard/ui/changelog.html diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html index ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 100644 --- a/perf/dashboard/ui/changelog.html +++ b/perf/dashboard/ui/changelog.html @@ -58,8 +58,8 @@ form { </form> <script> -var svnRootUrl = "http://src.chromium.org/svn/"; -var viewVc = "http://src.chromium.org/viewvc/chrome"; +var svnRootUrl = "//src.chromium.org/svn/"; +var viewVc = "//src.chromium.org/viewvc/chrome"; params = ParseParams(); var svnPath = params["url"];
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qyearsley@chromium.org/148963013/20001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for perf/dashboard/ui/changelog.html: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file perf/dashboard/ui/changelog.html Hunk #1 FAILED at 58. 1 out of 1 hunk FAILED -- saving rejects to file perf/dashboard/ui/changelog.html.rej Patch: perf/dashboard/ui/changelog.html Index: dashboard/ui/changelog.html diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html index ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 100644 --- a/perf/dashboard/ui/changelog.html +++ b/perf/dashboard/ui/changelog.html @@ -58,8 +58,8 @@ form { </form> <script> -var svnRootUrl = "http://src.chromium.org/svn/"; -var viewVc = "http://src.chromium.org/viewvc/chrome"; +var svnRootUrl = "//src.chromium.org/svn/"; +var viewVc = "//src.chromium.org/viewvc/chrome"; params = ParseParams(); var svnPath = params["url"];
The CQ bit was checked by qyearsley@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qyearsley@chromium.org/148963013/20001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for perf/dashboard/ui/changelog.html: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file perf/dashboard/ui/changelog.html Hunk #1 FAILED at 58. 1 out of 1 hunk FAILED -- saving rejects to file perf/dashboard/ui/changelog.html.rej Patch: perf/dashboard/ui/changelog.html Index: dashboard/ui/changelog.html diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html index ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 100644 --- a/perf/dashboard/ui/changelog.html +++ b/perf/dashboard/ui/changelog.html @@ -58,8 +58,8 @@ form { </form> <script> -var svnRootUrl = "http://src.chromium.org/svn/"; -var viewVc = "http://src.chromium.org/viewvc/chrome"; +var svnRootUrl = "//src.chromium.org/svn/"; +var viewVc = "//src.chromium.org/viewvc/chrome"; params = ParseParams(); var svnPath = params["url"];
On 2014/02/04 22:11:47, I haz the power (commit-bot) wrote: > Failed to apply patch for perf/dashboard/ui/changelog.html: > While running patch -p1 --forward --force --no-backup-if-mismatch; > patching file perf/dashboard/ui/changelog.html > Hunk #1 FAILED at 58. > 1 out of 1 hunk FAILED -- saving rejects to file > perf/dashboard/ui/changelog.html.rej > > Patch: perf/dashboard/ui/changelog.html > Index: dashboard/ui/changelog.html > diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html > index > ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 > 100644 > --- a/perf/dashboard/ui/changelog.html > +++ b/perf/dashboard/ui/changelog.html > @@ -58,8 +58,8 @@ form { > </form> > > <script> > -var svnRootUrl = "http://src.chromium.org/svn/"; > -var viewVc = "http://src.chromium.org/viewvc/chrome"; > +var svnRootUrl = "//src.chromium.org/svn/"; > +var viewVc = "//src.chromium.org/viewvc/chrome"; > > params = ParseParams(); > var svnPath = params["url"]; It looks like somehow this CL has already successfully been committed, even though it appears not to have gone through here. https://chromium.googlesource.com/chromium/tools/perf/+/7698ddcc29eb38e621a91... Does that seem right? If so, I can delete this CL now.
On 2014/02/04 23:37:13, qyearsley wrote: > On 2014/02/04 22:11:47, I haz the power (commit-bot) wrote: > > Failed to apply patch for perf/dashboard/ui/changelog.html: > > While running patch -p1 --forward --force --no-backup-if-mismatch; > > patching file perf/dashboard/ui/changelog.html > > Hunk #1 FAILED at 58. > > 1 out of 1 hunk FAILED -- saving rejects to file > > perf/dashboard/ui/changelog.html.rej > > > > Patch: perf/dashboard/ui/changelog.html > > Index: dashboard/ui/changelog.html > > diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html > > index > > > ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 > > 100644 > > --- a/perf/dashboard/ui/changelog.html > > +++ b/perf/dashboard/ui/changelog.html > > @@ -58,8 +58,8 @@ form { > > </form> > > > > <script> > > -var svnRootUrl = "http://src.chromium.org/svn/"; > > -var viewVc = "http://src.chromium.org/viewvc/chrome"; > > +var svnRootUrl = "//src.chromium.org/svn/"; > > +var viewVc = "//src.chromium.org/viewvc/chrome"; > > > > params = ParseParams(); > > var svnPath = params["url"]; > > It looks like somehow this CL has already successfully been committed, even > though it appears not to have gone through here. > https://chromium.googlesource.com/chromium/tools/perf/+/7698ddcc29eb38e621a91... > > Does that seem right? If so, I can delete this CL now. That has happened to me in the past as well. I assume you mean Close this CL, not delete? That sounds right to me. Maybe also paste a link to the revision it was committed as into the description (like Rietveld CLs usually gets when a CL is committed)
On 2014/02/05 07:01:56, Henrik Kjellander wrote: > On 2014/02/04 23:37:13, qyearsley wrote: > > On 2014/02/04 22:11:47, I haz the power (commit-bot) wrote: > > > Failed to apply patch for perf/dashboard/ui/changelog.html: > > > While running patch -p1 --forward --force --no-backup-if-mismatch; > > > patching file perf/dashboard/ui/changelog.html > > > Hunk #1 FAILED at 58. > > > 1 out of 1 hunk FAILED -- saving rejects to file > > > perf/dashboard/ui/changelog.html.rej > > > > > > Patch: perf/dashboard/ui/changelog.html > > > Index: dashboard/ui/changelog.html > > > diff --git perf/dashboard/ui/changelog.html perf/dashboard/ui/changelog.html > > > index > > > > > > ea6f3c520a1c3cde0b87f64b5e157b074c562204..4cced7ab1e5d0da0be33d6cc365b60ce2735bfe4 > > > 100644 > > > --- a/perf/dashboard/ui/changelog.html > > > +++ b/perf/dashboard/ui/changelog.html > > > @@ -58,8 +58,8 @@ form { > > > </form> > > > > > > <script> > > > -var svnRootUrl = "http://src.chromium.org/svn/"; > > > -var viewVc = "http://src.chromium.org/viewvc/chrome"; > > > +var svnRootUrl = "//src.chromium.org/svn/"; > > > +var viewVc = "//src.chromium.org/viewvc/chrome"; > > > > > > params = ParseParams(); > > > var svnPath = params["url"]; > > > > It looks like somehow this CL has already successfully been committed, even > > though it appears not to have gone through here. > > > https://chromium.googlesource.com/chromium/tools/perf/+/7698ddcc29eb38e621a91... > > > > Does that seem right? If so, I can delete this CL now. > > That has happened to me in the past as well. I assume you mean Close this CL, > not delete? That sounds right to me. Maybe also paste a link to the revision it > was committed as into the description (like Rietveld CLs usually gets when a CL > is committed) According to the revision log, this change was committed as 248779. http://src.chromium.org/viewvc/chrome/trunk/tools/perf/dashboard/ui/changelog...
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/185543008/ by stip@chromium.org. The reason for reverting is: This just got sync'd on the masters and it's causing errors for the revision ranges: Going here: http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?u... leads to a bad request here: http://build.chromium.org/cgi-bin/svn-log?url=//src.chromium.org/blink//trunk.... |