Stream: t-release/triage

Topic: 2020-10-12 to 10-18


view this post on Zulip DPC (Oct 12 2020 at 13:51):

Triage reports for the week

view this post on Zulip John Simon (Oct 13 2020 at 00:16):

S-waiting-on-author
https://github.com/rust-lang/rust/pull/73210 - apparently we're not supposed to triage mir
https://github.com/rust-lang/rust/pull/77148 - listed as "do not merge", ignoring
https://github.com/rust-lang/rust/pull/76723 - mir
https://github.com/rust-lang/rust/pull/77035 - broken build, bounced back to author mibac138
https://github.com/rust-lang/rust/pull/77027 - bounced back to author termhn to address comments
https://github.com/rust-lang/rust/pull/76219 - bounced back to author because conflicts but didn't ping because Mark-Simulacrum is a rust member

S-waiting-on-review
https://github.com/rust-lang/rust/pull/76931 - not triaging mir, but lots of conflicts
https://github.com/rust-lang/rust/pull/77099 - no one is assigned to this
https://github.com/rust-lang/rust/pull/76427 - no one seems to be sure about this, maybe close?
https://github.com/rust-lang/rust/pull/76392 - conversation seems to have stalled out...
https://github.com/rust-lang/rust/pull/77185 - waiting on a merge or further comments from jyn514
https://github.com/rust-lang/rust/pull/77031 - mir
https://github.com/rust-lang/rust/pull/75778 - approved by jyn514 but has a merge conflict now... should this go back to the author?
https://github.com/rust-lang/rust/pull/77213 - waiting on review from GuillaumeGomez
https://github.com/rust-lang/rust/pull/75023m - waiting on review from estebank
https://github.com/rust-lang/rust/pull/77300 - GuillaumeGomez commented 14 days ago: I'm fine with the current handling too. So if no one sees a point in having it, I'll just close this PR. :) .. closed

view this post on Zulip DPC (Oct 13 2020 at 00:27):

@John Simon

apparently we're not supposed to triage mir

can you clarify? do you mean ignore .mir files in conflicts?

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:10):

https://github.com/rust-lang/rust/pull/75778 - approved by jyn514 but has a merge conflict now... should this go back to the author?

yes, I marked it as waiting for author

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:11):

in particular they need to split the change linking to the rustdoc book with --help to a different PR, I'm not sure about that change and it should go with https://github.com/rust-lang/rust/pull/76427 which makes similar changes

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:12):

https://github.com/rust-lang/rust/pull/77185 - waiting on a merge or further comments from jyn514

personally I think it should just be closed

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:12):

I don't see the point of it

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:13):

https://github.com/rust-lang/rust/pull/76392 - conversation seems to have stalled out...

it's waiting on review from T-libs, the conversation seems to have agreed the change itself is fine as long as it doesnt' close the issue

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:14):

https://github.com/rust-lang/rust/pull/76427 - no one seems to be sure about this, maybe close?

personally I'd be ok with it (not strongly in favor) if they move the text down below the help

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:15):

but the author seems to have disappeared lately :( I think they got busy with life

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:15):

marked as waiting for author

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:18):

https://github.com/rust-lang/rust/pull/77213 - waiting on review from GuillaumeGomez

should we ping?

view this post on Zulip DPC (Oct 13 2020 at 01:22):

Joshua Nelson said:

https://github.com/rust-lang/rust/pull/77213 - waiting on review from GuillaumeGomez

should we ping?

@Joshua Nelson if you feel confident enough you can reassign it to yourself, or ping them (or preferably discuss it in the rustdoc channel)

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:23):

I'm behind enough on reviews :sweat_smile:

view this post on Zulip Joshua Nelson (Oct 13 2020 at 01:23):

also not familiar with this part of rustdoc

view this post on Zulip DPC (Oct 13 2020 at 01:23):

you can go ahead and ping them, or i'll be asking them about it tomorrow anyway so not a problem

view this post on Zulip John Simon (Oct 13 2020 at 02:31):

DPC said:

John Simon

apparently we're not supposed to triage mir

can you clarify? do you mean ignore .mir files in conflicts?

Right, a few weeks back there was a PR that had a number of .mir conflicts and I was told to ignore those. Also there are a few .diff files that appear as conflicts in PRs like https://github.com/rust-lang/rust/pull/76723

view this post on Zulip Noah Lev (Oct 16 2020 at 02:55):

Triage report!

review:
#76931 this is blocked; updated label
#76901 22 days since last update
#77031 has merge conflicts in mir diffs, so updated label but wasn't sure whether to ping
#77151 17 days
#76765 author needs to address reviews -> updated label, pinged
#76227 waiting on team review in proposed FCP, updated to waiting-on-team
#75209 16 days (I triaged this one two weeks ago :laughing:)
#77016 I think this is waiting on author, so updated label
#77373 looks close, has merge conflicts -> updated label, pinged
#77317 16 days
#77278 my own PR :) -- last pinged reviewer (estebank) 13 days ago, last review 18 days ago

author:
#77148 experimental PR; seems the experiment failed, so pinged author to ask if it should be closed
#76723 still has merge conflicts -- 20 days ago is when last merge conflict occured
#74024 not sure what's up here
#77333 seems like this broke stuff
#76130 wip, not sure what's up here
#77225 new review 15 days ago, pinged author

blocked:
#76446 asked to clarify what this is blocked on, Joshua Nelson clarified

team:
#71780 author is busy and is open to someone else taking this up
#76041 updated 5 weeks ago, has needs-fcp label -- not sure what to do

view this post on Zulip Joshua Nelson (Oct 16 2020 at 02:56):

#71780 author is busy and is open to someone else taking this up

FYI this is not waiting on author, this needs a redesign of the pattern API

view this post on Zulip Noah Lev (Oct 16 2020 at 02:56):

Yes, it's S-waiting-on-team

view this post on Zulip Joshua Nelson (Oct 16 2020 at 02:56):

'taking this up' is 'redesign how Pattern works', not cleaning up the PR

view this post on Zulip Noah Lev (Oct 16 2020 at 02:57):

Oh :embarrassed:

view this post on Zulip Noah Lev (Oct 16 2020 at 02:57):

I saw this and thought it meant "finish off the pr" :)

However, I'm back at university now for my senior year, so it'll be harder for me to work on this, so I'd like to pass off the work to take this off my plate.

view this post on Zulip Joshua Nelson (Oct 16 2020 at 02:58):

#76723 still has merge conflicts -- 20 days ago is when last merge conflict occured

would love to see this merged, it had pretty signficant perf benefits IIRC

view this post on Zulip Joshua Nelson (Oct 16 2020 at 03:01):

#75209 16 days (I triaged this one two weeks ago :laugh:)

this one is ready to go, I approved on behalf on estebank

view this post on Zulip Noah Lev (Oct 16 2020 at 03:01):

Just realized my emoji text was wrong since I wrote this in Vim :laughing:

view this post on Zulip Joshua Nelson (Oct 16 2020 at 03:02):

#76901 22 days since last update

shouldn't we ping BurntSushi ?

view this post on Zulip Noah Lev (Oct 16 2020 at 03:02):

I just r? them

view this post on Zulip Noah Lev (Oct 16 2020 at 03:03):

Joshua Nelson said:

#76723 still has merge conflicts -- 20 days ago is when last merge conflict occured

would love to see this merged, it had pretty signficant perf benefits IIRC

Should I ping Jonas Schievink? They're a Rust team member, so I think no, but wasn't sure...

view this post on Zulip Joshua Nelson (Oct 16 2020 at 03:04):

#76227 waiting on team review in proposed FCP, updated to waiting-on-team

are you sure that's right? It's still waiting for the FCP to complete it looks like

view this post on Zulip Joshua Nelson (Oct 16 2020 at 03:04):

Camelid said:

Joshua Nelson said:

#76723 still has merge conflicts -- 20 days ago is when last merge conflict occured

would love to see this merged, it had pretty signficant perf benefits IIRC

Should I ping Jonas Schievink? They're a Rust team member, so I think no, but wasn't sure...

normally no but I'm not part of triage so I will :P

view this post on Zulip Noah Lev (Oct 16 2020 at 03:04):

It entered FCP right after I triaged!

view this post on Zulip Noah Lev (Oct 16 2020 at 03:05):

And dtolnay just r+ed

view this post on Zulip Noah Lev (Oct 16 2020 at 03:06):

Does that do anything when it's waiting on FCP?

view this post on Zulip Joshua Nelson (Oct 16 2020 at 03:06):

https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/Don't.20stop.20writing.20copy-propagation.20passes

view this post on Zulip Charles Lew (Oct 16 2020 at 12:55):

AFK this week again.... Sorry have to skip one more week

view this post on Zulip DPC (Oct 16 2020 at 13:41):

no worries :smile: thanks

view this post on Zulip Léo Lanteri Thauvin (Oct 16 2020 at 20:12):

S-waiting-on-author

#77223: discussion stalled, maybe worth trying to bring reviewer's attention, but the PR is only code documentation so low priority?
#73928: I need to do something about this PR someday...

S-waiting-on-review

#77185: minor cleanup, reviewer last said "I still don't really understand the point of this change"
#76232: needs FCP, marked as S-waiting-on-team
#75551: assigned reviewer "not qualified to review this", libs team was pinged on 26 August + conflicts
#77334: been waiting without review, should probably be reassigned
#77317: waiting for an answer by eddyb
#77435: been waiting without any review yet
#75915: author addressed first "review", this is a very minor cleanup PR that I'd expect would get easily merged or rejected. Maybe pinging a reviewer would be reasonable here
#77326: needs T-libs FCP, marked as S-waiting-on-team
#76676: conflicts, marked as S-waiting-on-author and pinged author

view this post on Zulip Léo Lanteri Thauvin (Oct 16 2020 at 20:14):

Had some time to spare :D

view this post on Zulip Noah Lev (Oct 16 2020 at 20:23):

#75915: author addressed first "review", this is a very minor cleanup PR that I'd expect would get easily merged or rejected. Maybe pinging a reviewer would be reasonable here

Yeah, this is my PR. I don't feel super strongly about it, so it would be nice to get a review and either merge it or close it

view this post on Zulip DPC (Oct 16 2020 at 21:20):

thanks will look into it after some time

view this post on Zulip John Simon (Oct 19 2020 at 04:52):

S-waiting-on-author
https://github.com/rust-lang/rust/pull/77271 - listed as experiement, leaving it alone
https://github.com/rust-lang/rust/pull/75671 - pinged nathanwit to address review comments
https://github.com/rust-lang/rust/pull/76447 - pinged pickfire to address review comments
https://github.com/rust-lang/rust/pull/74942 - looks like waiting on review from joshtriplett for a month - though it also has a build failure, bouncing back to author MarinPostma
https://github.com/rust-lang/rust/pull/77431 - test fails, bouncing back to author
https://github.com/rust-lang/rust/pull/76696 - merge conflicts

S-waiting-on-review
https://github.com/rust-lang/rust/pull/77376 - looks like it's waiting on review from jyn514
https://github.com/rust-lang/rust/pull/77283 - looks like still waiting on review from matthewjasper
https://github.com/rust-lang/rust/pull/77268 - has approvals and is waiting on merge from spastorino
https://github.com/rust-lang/rust/pull/77420 - has approvals... looks like it should be merged

view this post on Zulip Joshua Nelson (Oct 19 2020 at 05:35):

https://github.com/rust-lang/rust/pull/77376 - looks like it's waiting on review from jyn514

this one makes me nervous :/

view this post on Zulip Joshua Nelson (Oct 19 2020 at 05:35):

if it has regressions it's possible they won't be caught by the test suite; it's changing the test suite itself

view this post on Zulip Joshua Nelson (Oct 19 2020 at 05:36):

the worst-case scenario is that it starts silently not testing things, and I don't know how to catch that

view this post on Zulip Joshua Nelson (Oct 19 2020 at 05:36):

nothing the author can really do either, other than writing a whole new framework for seeing what's run in CI and what isn't

view this post on Zulip Joshua Nelson (Oct 19 2020 at 05:38):

and it's big enough and changes enough rarely-run things that I'm not confident I caught everything in review

view this post on Zulip Noah Lev (Oct 19 2020 at 17:28):

Should we just close it then? That would also be unfortunate since the author is a first-time contributor and they followed the correct protocol of having an issue first :/

view this post on Zulip Joshua Nelson (Oct 19 2020 at 20:18):

well it's a good change :/

view this post on Zulip Joshua Nelson (Oct 19 2020 at 20:18):

I'm not sure what to do.

view this post on Zulip DPC (Oct 19 2020 at 20:40):

pr looks questionable to me, so i guess closing wouldn't be a bad thing

view this post on Zulip lzutao (Oct 20 2020 at 04:43):

@Joshua Nelson I'm sorry. I am the one opening the issue that #77376 wants to fix.
My intention is that the assigner/author would fix the issue by several PRs, not an all-in-one PR.

view this post on Zulip Joshua Nelson (Oct 20 2020 at 04:51):

Oh you don't have to be sorry, it's a good change. I just don't know what to do about it.

view this post on Zulip Pietro Albini (Oct 20 2020 at 08:44):

we should just switch CI away from bash

view this post on Zulip Pietro Albini (Oct 20 2020 at 08:44):

only half-joking

view this post on Zulip Léo Lanteri Thauvin (Oct 20 2020 at 08:46):

Out of curiosity, what would you replace it with?

view this post on Zulip Pietro Albini (Oct 20 2020 at 08:54):

my weird idea is to write all the ci setup stuff in rust

view this post on Zulip Léo Lanteri Thauvin (Oct 20 2020 at 08:55):

To make it somewhat more maintainable?

view this post on Zulip Pietro Albini (Oct 20 2020 at 09:01):

yeah

view this post on Zulip Noah Lev (Oct 20 2020 at 17:58):

Or at least Python. I'm impressed that T-infra is able to maintain bash -- it would be overwhelming to me


Last updated: Jan 29 2022 at 10:29 UTC