Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2946443003:
Milo: (Breaking proto change) Update console definition (Closed)
|
Created:
3 years, 6 months ago by Ryan Tseng Modified:
3 years, 5 months ago Reviewers:
iannucci CC:
chromium-reviews, infra-reviews+luci-go_chromium.org, maruel+w_chromium.org, tandrii+luci-go_chromium.org Target Ref:
refs/heads/master Project:
luci-go Visibility:
Public. |
DescriptionMilo: (Breaking proto change) Update console definition
This is a breaking proto refactor for the console definition.
* Module is no longer a separate field, it's just a part of the full builder path
* Remove ACLs, these are done via project.cfg now.
* Add ManifestName, which is the name of the checkout
BUG=468053
Review-Url: https://codereview.chromium.org/2946443003
Committed: https://github.com/luci/luci-go/commit/b8f6eb4b659ad1cd4511d0e6320fe3c7d9fe4a3b
Patch Set 1 #
Total comments: 4
Patch Set 2 : Rebase #Patch Set 3 : Rebase #Patch Set 4 : Rebase/Retrain #Patch Set 5 : ImportFix #
Messages
Total messages: 42 (33 generated)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Milo: (Breaking proto change) Update console definition This is a breaking proto refactor for the console definition. * Module is no longer a separate field, it's just a part of the full builder path * Remove ACLs, these are done via project.cfg now. BUG= ========== to ========== Milo: (Breaking proto change) Update console definition This is a breaking proto refactor for the console definition. * Module is no longer a separate field, it's just a part of the full builder path * Remove ACLs, these are done via project.cfg now. BUG=468053 ==========
hinoka@google.com changed reviewers: + iannucci@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Milo: (Breaking proto change) Update console definition This is a breaking proto refactor for the console definition. * Module is no longer a separate field, it's just a part of the full builder path * Remove ACLs, these are done via project.cfg now. BUG=468053 ========== to ========== Milo: (Breaking proto change) Update console definition This is a breaking proto refactor for the console definition. * Module is no longer a separate field, it's just a part of the full builder path * Remove ACLs, these are done via project.cfg now. * Add ManifestName, which is the name of the checkout BUG=468053 ==========
Console def is in project.proto
lgtm, though I have Questions https://codereview.chromium.org/2946443003/diff/1/milo/common/config/project.... File milo/common/config/project.proto (right): https://codereview.chromium.org/2946443003/diff/1/milo/common/config/project.... milo/common/config/project.proto:54: string Category = 2; Is this a good way to do this? IIUC, this means you'd do "chromium|chromium.win|Windows ThingBuilder"? Why not a repeated string? https://codereview.chromium.org/2946443003/diff/1/milo/common/config/project.... milo/common/config/project.proto:57: string ShortName = 3; what is this for? does this need to be unique?
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2946443003/diff/1/milo/common/config/project.... File milo/common/config/project.proto (right): https://codereview.chromium.org/2946443003/diff/1/milo/common/config/project.... milo/common/config/project.proto:54: string Category = 2; On 2017/06/17 17:13:29, iannucci wrote: > Is this a good way to do this? IIUC, this means you'd do > "chromium|chromium.win|Windows ThingBuilder"? > > Why not a repeated string? Buildbot does this. Was split originally on repeated string vs "|" separator. Went with | because it seemed easier to write, but either way is fine by me. https://codereview.chromium.org/2946443003/diff/1/milo/common/config/project.... milo/common/config/project.proto:57: string ShortName = 3; On 2017/06/17 17:13:29, iannucci wrote: > what is this for? does this need to be unique? For display only. Doesn't need to be unique.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hinoka@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from iannucci@chromium.org Link to the patchset: https://codereview.chromium.org/2946443003/#ps20001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/36e5cb0ff181d710)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/373845af36ad7110)
The CQ bit was checked by hinoka@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from iannucci@chromium.org Link to the patchset: https://codereview.chromium.org/2946443003/#ps40001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/374752357efcbf10)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/37475a98add42c10) Luci-go Mac Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/37475a98b2878310)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hinoka@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from iannucci@chromium.org Link to the patchset: https://codereview.chromium.org/2946443003/#ps80001 (title: "ImportFix")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499725339937450,
"parent_rev": "20a1165ab0ef98cce14e4eac859879a4630abc92", "commit_rev":
"b8f6eb4b659ad1cd4511d0e6320fe3c7d9fe4a3b"}
Message was sent while issue was closed.
Description was changed from ========== Milo: (Breaking proto change) Update console definition This is a breaking proto refactor for the console definition. * Module is no longer a separate field, it's just a part of the full builder path * Remove ACLs, these are done via project.cfg now. * Add ManifestName, which is the name of the checkout BUG=468053 ========== to ========== Milo: (Breaking proto change) Update console definition This is a breaking proto refactor for the console definition. * Module is no longer a separate field, it's just a part of the full builder path * Remove ACLs, these are done via project.cfg now. * Add ManifestName, which is the name of the checkout BUG=468053 Review-Url: https://codereview.chromium.org/2946443003 Committed: https://github.com/luci/luci-go/commit/b8f6eb4b659ad1cd4511d0e6320fe3c7d9fe4a3b ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://github.com/luci/luci-go/commit/b8f6eb4b659ad1cd4511d0e6320fe3c7d9fe4a3b |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
