Stream: t-compiler/wg-nll

Topic: Felix wrestles with Github


pnkfelix (Oct 08 2018 at 09:55, on Zulip):

its not a big deal; I wrote the comment on a couple PR's but then the comment would get outdated by an update to a the PR

pnkfelix (Oct 08 2018 at 09:55, on Zulip):

and so it would get hidden

pnkfelix (Oct 08 2018 at 09:55, on Zulip):

I'll go find the cases where I can remember it

davidtwco (Oct 08 2018 at 09:56, on Zulip):

Ah, I always try to make sure I respond to every comment, I’ll try double check that more in future.

pnkfelix (Oct 08 2018 at 09:57, on Zulip):

Pending ?

pnkfelix (Oct 08 2018 at 09:57, on Zulip):

hmm

pnkfelix (Oct 08 2018 at 09:57, on Zulip):

This was the second one I wrote; I'm still looking for the first: https://github.com/rust-lang/rust/pull/54831/files#diff-f5a7553f1d02fe100fab92a0f135d0fdR230

davidtwco (Oct 08 2018 at 09:57, on Zulip):

There’s no comment there.

davidtwco (Oct 08 2018 at 09:58, on Zulip):

If it’s pending you didn’t submit the review.

pnkfelix (Oct 08 2018 at 09:59, on Zulip):

wait so comments I write don't show up??

pnkfelix (Oct 08 2018 at 09:59, on Zulip):

argh!

pnkfelix (Oct 08 2018 at 09:59, on Zulip):

/me angry

pnkfelix (Oct 08 2018 at 10:00, on Zulip):

That's really frustrating

pnkfelix (Oct 08 2018 at 10:01, on Zulip):

maybe it makes sense in terms of overall work flow, but the fact that this note is not actually registered for anyone else to see, and that the feedback to me about that is just this Pending label (which I'm not certain even shows up in the mobile phone interface ...)

pnkfelix (Oct 08 2018 at 10:02, on Zulip):

I'm now wondering how many comments I've made have been silently discarded due to this...

pnkfelix (Oct 08 2018 at 10:03, on Zulip):

anyway, sorry, my own github fails are obviously not your problem.

davidtwco (Oct 08 2018 at 10:03, on Zulip):

If you put a comment in I'd always respond and try resolve it.

pnkfelix (Oct 08 2018 at 10:04, on Zulip):

right, but I just didn't know that my comments were being held by github as part of its review system

pnkfelix (Oct 08 2018 at 10:04, on Zulip):

I understood that it presented me with this embedded choice-box of "approve, reject, etc"

pnkfelix (Oct 08 2018 at 10:05, on Zulip):

but I assumed that if I didn't make any choice, that my comment would still show up in the meantime and be visible

davidtwco (Oct 08 2018 at 10:05, on Zulip):

Normally the "no choice" option is neither approve or reject but just "comment" in that little box.

pnkfelix (Oct 08 2018 at 10:06, on Zulip):

I now understand that I need to either hit submit (with "no choice") immediately. (That, or I need to finish reading the PR and then make a choice, of course).

pnkfelix (Oct 08 2018 at 10:06, on Zulip):

but I just didn't know that the way I was using it was broken. Because I don't, you know, visit github issues without being logged in as me

davidtwco (Oct 08 2018 at 10:06, on Zulip):

Yeah, it's easy to miss.

pnkfelix (Oct 08 2018 at 10:06, on Zulip):

anyway I've now identified both the cases where I made the comments and have now submitted them.

davidtwco (Oct 08 2018 at 10:07, on Zulip):

Great, I'll get those just now.

davidtwco (Oct 08 2018 at 21:17, on Zulip):

w/r/t your comment from earlier - I think it depends when making the review whether you press "add a comment" or "start a review". One of them will add a pending comment to be later be submitted with others all at once; the other will just add it straight away.

nikomatsakis (Oct 08 2018 at 21:18, on Zulip):

it would be nice if there were some indication on the PR of a "review in progress"

davidtwco (Oct 08 2018 at 21:19, on Zulip):

Somewhat related, one thing that GitLab does which is nice is in the issue listing, it has a little icon if there's a PR up for that issue. Would be nice for something similar on GitHub, but also a similar icon on PRs for in-progress reviews.

lqd (Oct 09 2018 at 08:31, on Zulip):

@davidtwco the refined github extension offers this. Here's an example from one of yours: next to "Open" is the PR number which will close this issue #53040

davidtwco (Oct 09 2018 at 08:48, on Zulip):

Last time I tried that it made some things look strange so I uninstalled it, might be worth another go. I always wanted this extension to be available for Firefox.

Last update: Nov 22 2019 at 00:40UTC