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

Issue 2153513002: [Telemetry] Ensure that story display names are unique (Closed)

Created:
4 years ago by eakuefner
Modified:
3 years, 8 months ago
Reviewers:
nednguyen
CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org
Base URL:
git@github.com:catapult-project/catapult.git@master
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Fix a bunch of tests #

Patch Set 3 : Address Ned's comment #

Patch Set 4 : rebase #

Patch Set 5 : Use set-based check #

Total comments: 4

Patch Set 6 : Address Ned's comment #

Patch Set 7 : fix typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+96 lines, -31 lines) Patch
M telemetry/telemetry/internal/story_runner_unittest.py View 1 2 3 8 chunks +19 lines, -18 lines 0 comments Download
M telemetry/telemetry/page/page_run_end_to_end_unittest.py View 1 2 3 3 chunks +12 lines, -7 lines 0 comments Download
M telemetry/telemetry/story/story.py View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M telemetry/telemetry/story/story_set.py View 1 2 3 4 5 6 3 chunks +20 lines, -4 lines 0 comments Download
M telemetry/telemetry/story/story_set_unittest.py View 1 2 3 4 5 2 chunks +42 lines, -2 lines 0 comments Download

Messages

Total messages: 67 (31 generated)
eakuefner
Please, take a look.
4 years ago (2016-07-14 18:52:01 UTC) #2
nednguyen
lgtm Thanks!
4 years ago (2016-07-14 19:01:18 UTC) #3
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/2153513002/1
4 years ago (2016-07-14 23:25:18 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Mac%20Tryserver/builds/3968)
4 years ago (2016-07-14 23:40:13 UTC) #7
nednguyen
lgtm but as we talked offline, what's unique here is story name + grouping key
4 years ago (2016-07-15 16:34:12 UTC) #8
eakuefner
On 2016/07/15 at 16:34:12, nednguyen wrote: > lgtm but as we talked offline, what's unique ...
4 years ago (2016-07-15 19:34:58 UTC) #9
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/2153513002/40001
4 years ago (2016-07-15 19:35:16 UTC) #12
eakuefner
Actually, I will land this on Monday because I would like to watch the Catapult ...
4 years ago (2016-07-15 19:35:43 UTC) #14
nednguyen
On 2016/07/15 19:35:43, eakuefner wrote: > Actually, I will land this on Monday because I ...
4 years ago (2016-07-15 19:38:38 UTC) #15
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/2153513002/40001
4 years ago (2016-07-18 17:27:48 UTC) #17
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/42560ff37c155a0b8c319f79511154f58c465b34
4 years ago (2016-07-18 17:52:13 UTC) #19
commit-bot: I haz the power
CQ bit was unchecked.
4 years ago (2016-07-18 17:52:15 UTC) #20
eakuefner
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2163583002/ by eakuefner@chromium.org. ...
4 years ago (2016-07-19 06:28:05 UTC) #21
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/2153513002/60001
3 years, 9 months ago (2016-10-28 19:00:35 UTC) #28
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/a3d2a9721a0bc501733c7e03722e902a7d7c0077
3 years, 9 months ago (2016-10-28 19:30:27 UTC) #30
eakuefner
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2457303003/ by eakuefner@chromium.org. ...
3 years, 9 months ago (2016-10-29 04:25:06 UTC) #31
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/2153513002/60001
3 years, 8 months ago (2016-12-01 00:13:39 UTC) #33
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/1b820a28c383840a84933543dd4630daa0f25fc0
3 years, 8 months ago (2016-12-01 01:02:19 UTC) #36
eakuefner
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2545573003/ by eakuefner@chromium.org. ...
3 years, 8 months ago (2016-12-01 04:00:24 UTC) #37
nednguyen
On 2016/12/01 04:00:24, eakuefner wrote: > A revert of this CL (patchset #4 id:60001) has ...
3 years, 8 months ago (2016-12-01 13:08:33 UTC) #38
eakuefner
On 2016/12/01 at 13:08:33, nednguyen wrote: > On 2016/12/01 04:00:24, eakuefner wrote: > > A ...
3 years, 8 months ago (2016-12-01 15:57:53 UTC) #39
nednguyen
On 2016/12/01 15:57:53, eakuefner wrote: > On 2016/12/01 at 13:08:33, nednguyen wrote: > > On ...
3 years, 8 months ago (2016-12-01 16:00:52 UTC) #40
eakuefner
On 2016/12/01 at 16:00:52, nednguyen wrote: > On 2016/12/01 15:57:53, eakuefner wrote: > > On ...
3 years, 8 months ago (2016-12-01 17:18:42 UTC) #41
nednguyen
On 2016/12/01 17:18:42, eakuefner wrote: > On 2016/12/01 at 16:00:52, nednguyen wrote: > > On ...
3 years, 8 months ago (2016-12-02 12:15:14 UTC) #42
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/2153513002/80001
3 years, 8 months ago (2016-12-02 19:48:45 UTC) #45
nednguyen
https://codereview.chromium.org/2153513002/diff/80001/telemetry/telemetry/story/story_set.py File telemetry/telemetry/story/story_set.py (right): https://codereview.chromium.org/2153513002/diff/80001/telemetry/telemetry/story/story_set.py#newcode35 telemetry/telemetry/story/story_set.py:35: self.story_names_and_grouping_keys = set() Can you make this private? Things ...
3 years, 8 months ago (2016-12-02 19:59:58 UTC) #47
nednguyen
https://codereview.chromium.org/2153513002/diff/80001/telemetry/telemetry/story/story_set.py File telemetry/telemetry/story/story_set.py (right): https://codereview.chromium.org/2153513002/diff/80001/telemetry/telemetry/story/story_set.py#newcode118 telemetry/telemetry/story/story_set.py:118: self.stories.remove(story) Don't you need to remove the corresponding data ...
3 years, 8 months ago (2016-12-02 20:00:57 UTC) #48
eakuefner
Thanks for your comments. Please take another look. https://codereview.chromium.org/2153513002/diff/80001/telemetry/telemetry/story/story_set.py File telemetry/telemetry/story/story_set.py (right): https://codereview.chromium.org/2153513002/diff/80001/telemetry/telemetry/story/story_set.py#newcode35 telemetry/telemetry/story/story_set.py:35: self.story_names_and_grouping_keys ...
3 years, 8 months ago (2016-12-02 20:28:58 UTC) #49
nednguyen
lgtm
3 years, 8 months ago (2016-12-02 20:59:38 UTC) #52
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/2153513002/120001
3 years, 8 months ago (2016-12-02 21:13:30 UTC) #57
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Linux%20Tryserver/builds/5791)
3 years, 8 months ago (2016-12-02 22:14:26 UTC) #59
eakuefner
Kari, it looks like there are some issues on Linux to do with "can't find ...
3 years, 8 months ago (2016-12-02 22:24:17 UTC) #60
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/2153513002/120001
3 years, 8 months ago (2016-12-02 22:33:29 UTC) #62
aiolos (Not reviewing)
On 2016/12/02 22:24:17, eakuefner wrote: > Kari, it looks like there are some issues on ...
3 years, 8 months ago (2016-12-02 22:50:40 UTC) #63
aiolos (Not reviewing)
On 2016/12/02 22:50:40, aiolos wrote: > On 2016/12/02 22:24:17, eakuefner wrote: > > Kari, it ...
3 years, 8 months ago (2016-12-02 23:00:47 UTC) #64
commit-bot: I haz the power
3 years, 8 months ago (2016-12-02 23:25:34 UTC) #67
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld 408576698