Make SVN commit number an optional part of X-Mod-Pagespeed header.

Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".

BUG=660950
Committed: https://crrev.com/b3028617e93ab30b34c34f5e12008b6e171f909e
Cr-Commit-Position: refs/heads/master@{#429075}

[cfredric (do not use), 2016-10-31 20:14:54]

Description was changed from

==========
Make SVN commit number an optional part of X-Mod-Pagespeed header.

BUG=
==========

to

==========
Make SVN commit number an optional part of X-Mod-Pagespeed header.

BUG=
==========

[cfredric (do not use), 2016-10-31 20:14:55]

cfredric@google.com changed reviewers:
+ csharrison@chromium.org

[Charlie Harrison, 2016-10-31 20:29:00]

Looks good!

Could you:
1. Update the BUG= line with a bug # from crbug.com.
2. Write a short description for this change.

Keep in mind the description and title of the CL should be wrapped to 80 cols.

https://codereview.chromium.org/2465023002/diff/20001/chrome/browser/mod_page...
File chrome/browser/mod_pagespeed/mod_pagespeed_metrics.cc (right):

https://codereview.chromium.org/2465023002/diff/20001/chrome/browser/mod_page...
chrome/browser/mod_pagespeed/mod_pagespeed_metrics.cc:46: int num_parsed =
sscanf(version_string, "%d.%d.%d.%d-%d", &unused_major,
I wonder if you could do
sscanf(version_string, "%d.%d.%d.%d%c%d"), and if we match 6 successfully, just
double check that the character was a '-'. This way if we don't have the commit
we don't have to re-parse the version string.

[cfredric (do not use), 2016-10-31 20:43:00]

Description was changed from

==========
Make SVN commit number an optional part of X-Mod-Pagespeed header.

BUG=
==========

to

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".

BUG=660950
==========

[cfredric (do not use), 2016-10-31 20:44:44]

Description was changed from

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".

BUG=660950
==========

to

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".


BUG=660950
==========

[cfredric (do not use), 2016-10-31 20:54:21]

On 2016/10/31 20:29:00, Charlie Harrison wrote:
> Looks good!
> 
> Could you:
> 1. Update the BUG= line with a bug # from http://crbug.com.
> 2. Write a short description for this change.
> 
> Keep in mind the description and title of the CL should be wrapped to 80 cols.
> 
>
https://codereview.chromium.org/2465023002/diff/20001/chrome/browser/mod_page...
> File chrome/browser/mod_pagespeed/mod_pagespeed_metrics.cc (right):
> 
>
https://codereview.chromium.org/2465023002/diff/20001/chrome/browser/mod_page...
> chrome/browser/mod_pagespeed/mod_pagespeed_metrics.cc:46: int num_parsed =
> sscanf(version_string, "%d.%d.%d.%d-%d", &unused_major,
> I wonder if you could do
> sscanf(version_string, "%d.%d.%d.%d%c%d"), and if we match 6 successfully,
just
> double check that the character was a '-'. This way if we don't have the
commit
> we don't have to re-parse the version string.

Done.

[Charlie Harrison, 2016-10-31 21:01:03]

lgtm!

[cfredric (do not use), 2016-11-01 13:47:51]

cfredric@google.com changed reviewers:
+ sky@chromium.org

[sky, 2016-11-01 15:26:01]

sky@chromium.org changed reviewers:
+ davidben@chromium.org

[sky, 2016-11-01 15:26:01]

I'm not at all familiar with this code. It looks like davidben moved it.
Hopefully he can review this.

[davidben, 2016-11-01 15:36:38]

lgtm deferring to csharrison. (This code is unowned, sadly. Hopefully cfredric
and co can take up ownership of it since it's for their stuff. My only
involvement was moving it out of chrome/browser/prerender, where it was put
because the original author just happened to OWNERS bits there.)

[cfredric (do not use), 2016-11-01 15:44:35]

The CQ bit was checked by cfredric@google.com

[cfredric (do not use), 2016-11-01 15:44:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
csharrison@chromium.org
Link to the patchset: https://codereview.chromium.org/2465023002/#ps60001
(title: "Simplify format string.")

[commit-bot: I haz the power, 2016-11-01 15:45:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 15:52:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-01 15:52:31]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2016-11-01 16:16:12]

Can you please add an OWNERS file in this patch?

[cfredric (do not use), 2016-11-01 16:50:07]

On 2016/11/01 16:16:12, sky wrote:
> Can you please add an OWNERS file in this patch?

Sorry about that - done.

[Charlie Harrison, 2016-11-01 16:50:58]

still lgtm

[Charlie Harrison, 2016-11-01 17:00:06]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 17:00:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 17:56:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-01 17:56:46]

Dry run: This issue passed the CQ dry run.

[sky, 2016-11-01 19:24:48]

LGTM

[cfredric (do not use), 2016-11-01 19:35:29]

The CQ bit was checked by cfredric@google.com

[cfredric (do not use), 2016-11-01 19:35:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/2465023002/#ps80001
(title: "Add mod_pagespeed/OWNERS file.")

[commit-bot: I haz the power, 2016-11-01 19:36:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 19:41:05]

Description was changed from

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".


BUG=660950
==========

to

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".


BUG=660950
==========

[commit-bot: I haz the power, 2016-11-01 19:41:06]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-01 19:57:37]

Description was changed from

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".


BUG=660950
==========

to

==========
Make the SVN commit number an optional part of X-Mod-Pagespeed header.
Currently, the X-Mod-Pagespeed header format is expected to be
"<major>.<minor>.<branch>.<point>-<commit>", where <commit> is the SVN commit
number. This should not be required; i.e. the format should also be allowed to
be "<major>.<minor>.<branch>.<point>".

BUG=660950

Committed: https://crrev.com/b3028617e93ab30b34c34f5e12008b6e171f909e
Cr-Commit-Position: refs/heads/master@{#429075}
==========

[commit-bot: I haz the power, 2016-11-01 19:57:38]

Patchset 5 (id:??) landed as
https://crrev.com/b3028617e93ab30b34c34f5e12008b6e171f909e
Cr-Commit-Position: refs/heads/master@{#429075}