[package.proto] add canonical_base_url field.

[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

NOTE: This changes the tag values for the proto definition... however, since 
we've never used the binary form of the package message, it shouldn't be a 
problem. This allows canonical base url to sort above 'deps'. Otherwise it gets
sorted to the bottom of the file, which is weird.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2728303004
Committed: https://github.com/luci/recipes-py/commit/22e413ad35481ecd49d232620e7794ce6f544958

[iannucci, 2017-03-07 02:38:58]

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

[commit-bot: I haz the power, 2017-03-07 02:39:03]

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, 2017-03-07 02:42:32]

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

[commit-bot: I haz the power, 2017-03-07 02:42:33]

Dry run: This issue passed the CQ dry run.

[Paweł Hajdan Jr., 2017-03-07 17:51:19]

I'm not sure why we involve protos here. It may be introducing a layer of
unnecessary complexity.

Please consider starting a doc for comments: what problem this is solving, and
what are advantages of doing this with protos.

I don't want to block you though, so if you have a strong preference, feel free
to move forward with other US reviewers.

[iannucci, 2017-03-07 18:05:06]

On 2017/03/07 17:51:19, Paweł Hajdan Jr. wrote:
> I'm not sure why we involve protos here. It may be introducing a layer of
> unnecessary complexity.
> 
> Please consider starting a doc for comments: what problem this is solving, and
> what are advantages of doing this with protos.
> 
> I don't want to block you though, so if you have a strong preference, feel
free
> to move forward with other US reviewers.

This CL is just updating existing protos.

[martiniss, 2017-03-07 18:56:38]

lgtm

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto
File recipe_engine/package.proto (right):

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:23: // The git commit ID that we depend on.
commit ID? Is that the technical term? I would have said revision hash, but you
might know git better than me :)

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:70: }
Shouldn't you update the recipes.cfg in here with this?

[dnj, 2017-03-07 18:58:53]

lgtm w/ some nits

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto
File recipe_engine/package.proto (right):

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:17: // The url of where to fetch the package data.
Must always be a git repo url.
Let's capitalize things like "URL" and "ID" :)

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:23: // The git commit ID that we depend on.
Is the correct term a commit ID or a commit?

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:35: // dependency, and GITILES will pull the data
via the GITILES REST API.
Put these per-enum docs up by the enums themselves.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:43: // The "API Version" of this proto. Should
always equal 1, currently. This may
Document that this MUST always have a tag value of "1" even if it is changed
incompatibly.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:49: // (e.g.  DEPS=["recipe_engine/path"]).
Maybe mention naming constraints (e.g., no slashes, etc.)

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:52: // The path (using POSIX-style slashes) to where
the base of the recipes are
Just say "forward slashes".

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:67: //   * Github
nit: GitHub

[iannucci, 2017-03-08 03:23:11]

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

[commit-bot: I haz the power, 2017-03-08 03:23:16]

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

[iannucci, 2017-03-08 03:28:37]

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto
File recipe_engine/package.proto (right):

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:17: // The url of where to fetch the package data.
Must always be a git repo url.
On 2017/03/07 18:58:53, dnj wrote:
> Let's capitalize things like "URL" and "ID" :)

Done.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:23: // The git commit ID that we depend on.
On 2017/03/07 18:58:52, dnj wrote:
> Is the correct term a commit ID or a commit?

both, I think. `object identity` is the sha1. 'commit' is fine though.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:35: // dependency, and GITILES will pull the data
via the GITILES REST API.
On 2017/03/07 18:58:52, dnj wrote:
> Put these per-enum docs up by the enums themselves.

Done.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:43: // The "API Version" of this proto. Should
always equal 1, currently. This may
On 2017/03/07 18:58:52, dnj wrote:
> Document that this MUST always have a tag value of "1" even if it is changed
> incompatibly.

Done.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:49: // (e.g.  DEPS=["recipe_engine/path"]).
On 2017/03/07 18:58:52, dnj wrote:
> Maybe mention naming constraints (e.g., no slashes, etc.)

Done.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:52: // The path (using POSIX-style slashes) to where
the base of the recipes are
On 2017/03/07 18:58:52, dnj wrote:
> Just say "forward slashes".

Done.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:67: //   * Github
On 2017/03/07 18:58:52, dnj wrote:
> nit: GitHub

Done.

https://codereview.chromium.org/2728303004/diff/1/recipe_engine/package.proto...
recipe_engine/package.proto:70: }
On 2017/03/07 18:56:38, martiniss wrote:
> Shouldn't you update the recipes.cfg in here with this?

Done.

[dnj, 2017-03-08 03:28:59]

https://codereview.chromium.org/2728303004/diff/20001/recipe_engine/package.py
File recipe_engine/package.py (right):

https://codereview.chromium.org/2728303004/diff/20001/recipe_engine/package.p...
recipe_engine/package.py:639: RootRepoSpec(proto_file))
nit: two more spaces

[commit-bot: I haz the power, 2017-03-08 03:29:46]

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

[commit-bot: I haz the power, 2017-03-08 03:29:47]

Dry run: Try jobs failed on following builders:
  Recipes-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/34c4c5f23f494010)

[iannucci, 2017-03-08 03:32:40]

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

[commit-bot: I haz the power, 2017-03-08 03:32:42]

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

[iannucci, 2017-03-08 03:33:31]

https://codereview.chromium.org/2728303004/diff/20001/recipe_engine/package.py
File recipe_engine/package.py (right):

https://codereview.chromium.org/2728303004/diff/20001/recipe_engine/package.p...
recipe_engine/package.py:639: RootRepoSpec(proto_file))
On 2017/03/08 03:28:59, dnj wrote:
> nit: two more spaces

Done.

[iannucci, 2017-03-08 03:35:46]

Description was changed from

==========
[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=
==========

to

==========
[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

NOTE: This changes the tag values for the proto definition... however, since
we've never used the binary form of the package message, it shouldn't be a
problem. This allows canonical base url to sort above 'deps'. Otherwise it gets
sorted to the bottom of the file, which is weird.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=
==========

[iannucci, 2017-03-08 03:36:02]

Description was changed from

==========
[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

NOTE: This changes the tag values for the proto definition... however, since
we've never used the binary form of the package message, it shouldn't be a
problem. This allows canonical base url to sort above 'deps'. Otherwise it gets
sorted to the bottom of the file, which is weird.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=
==========

to

==========
[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

NOTE: This changes the tag values for the proto definition... however, since 
we've never used the binary form of the package message, it shouldn't be a 
problem. This allows canonical base url to sort above 'deps'. Otherwise it gets
sorted to the bottom of the file, which is weird.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=
==========

[iannucci, 2017-03-08 03:36:11]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2017-03-08 03:36:11]

The patchset sent to the CQ was uploaded after l-g-t-m from dnj@chromium.org,
martiniss@chromium.org
Link to the patchset: https://codereview.chromium.org/2728303004/#ps80001
(title: "spaces")

[commit-bot: I haz the power, 2017-03-08 03:36:23]

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, 2017-03-08 03:39:38]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488944171205610,
"parent_rev": "928cf06d73113b2b349aaeb0b13d94e30ea3f0e1", "commit_rev":
"22e413ad35481ecd49d232620e7794ce6f544958"}

[commit-bot: I haz the power, 2017-03-08 03:39:41]

Description was changed from

==========
[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

NOTE: This changes the tag values for the proto definition... however, since 
we've never used the binary form of the package message, it shouldn't be a 
problem. This allows canonical base url to sort above 'deps'. Otherwise it gets
sorted to the bottom of the file, which is weird.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=
==========

to

==========
[package.proto] add canonical_base_url field.

Also document the fields in package.proto and re-run protoc.

canonical_base_url will be used when generating new Markdown documentation
to allow for meaningful hyperlinks. It's also generally a good idea so when
packages get mirrored, they'll always point to the source of truth.

NOTE: This changes the tag values for the proto definition... however, since 
we've never used the binary form of the package message, it shouldn't be a 
problem. This allows canonical base url to sort above 'deps'. Otherwise it gets
sorted to the bottom of the file, which is weird.

R=dnj@chromium.org, martiniss@chromium.org, phajdan.jr@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2728303004
Committed:
https://github.com/luci/recipes-py/commit/22e413ad35481ecd49d232620e7794ce6f5...
==========

[commit-bot: I haz the power, 2017-03-08 03:39:42]

Committed patchset #5 (id:80001) as
https://github.com/luci/recipes-py/commit/22e413ad35481ecd49d232620e7794ce6f5...

[iannucci, 2017-03-08 04:23:37]

A revert of this CL (patchset #5 id:80001) has been created in
https://chromiumcodereview.appspot.com/2739803002/ by iannucci@chromium.org.

The reason for reverting is: aand kitchen needs to handle this proto.
reverting..