Fancy links for GH issues representing external reviews #3749
Conversation
jyasskin
force-pushed
the
external-review-fancy-links
branch
from
3d2d891 to
7a9b972
Compare
| if (stateVariant === undefined) { | ||
| if (state === 'open') { | ||
| const age = Date.now() - createdAt.getTime(); | ||
| stateDescription = html`Opened <relative-time .date=${createdAt} format="relative" precision="day" threshold="P100Y">on ${_dateTimeFormat.format(createdAt)}</relative-time>`; |
There was a problem hiding this comment.
We already have <sl-relative-time> as used in utils.js. Could you use that? It might even just be wrapping the same underlying library.
There was a problem hiding this comment.
<sl-relative-time> is missing the precision argument that I'm using to turn "Opened 2 Hours Ago" into "Opened Today" (once github/relative-time-element#280 goes in), but that's not a big deal if you'd rather avoid the dependency. I'll see if there are any other problems once my development machine finishes upgrading.
There was a problem hiding this comment.
I have to undo the style from
chromium-dashboard/client-src/css/shared-css.js
Lines 191 to 193 in cffa571
but then it seems to work:
Here's a patch without <relative-time>.
jyasskin
force-pushed
the
external-review-fancy-links
branch
from
7a9b972 to
3a10f32
Compare
Hmm, this is trickier to consider than I expected! I do see the ~one month guidance for TAG, but not for other vendors, FWIW. I think the colors you have do make sense, once we document them. Red: "probably too soon for a fulsome response", orange: "proceed with caution" and gray/black: "YOLO 💀". Did I get that right?
This looks great. |
|
@miketaylr That's about right. I'd s/YOLO/the API owners have to make a call/ though. ;) https://www.chromium.org/blink/launching-features/#new-feature-prepare-to-ship now asks for 1 month for vendors too. |
It isn't up to us how long other vendors or the TAG take to respond though. Is this meant to represent whether the issue itself is overdue, or something whether a feature owner should consider it "timed out"? |
|
It's more about whether the external reviewer has clearly not had enough time to review it. The color here only goes to neutral gray after a month and not green because I don't think there's any automated way to decide whether the review has timed out. That's up to the human reviewers. |
Correct. But I think guidance to ask early enough to improve the odds of getting a response is good. And then API OWNERs can make a call (as we sometimes do "let's give it a week or two to bake and hope for a response", etc.) |
|
Ok makes aense. How about yellow for less than 1 months, meaning "in progress, shipping might block on it"? And after that gray. |
|
Works for me. I'll make that change Monday. |
jyasskin
force-pushed
the
external-review-fancy-links
branch
from
0142e78 to
3d573d2
Compare
|
Ok, everything less than a month (actually 4 weeks) is now yellow. I'll merge in a couple hours unless I hear concerns. Thanks! |
For all open issues, this adds the amount of time they've been open:
For open position issues, colors the links according to how long they've been open, with thresholds at 2 weeks and 1 month. https://www.chromium.org/blink/launching-features/#new-feature-prepare-to-ship says to give reviewers at least a month to review, so ... @chrishtr @miketaylr what colors do you want for what ages?
For resolved position issues, colors the links according to the position:
