8 ms·

Redesigned Conversations

126 points by bencevans 13y ago

urbanachiever 13y ago
Brings a lot of clarity to what used to be a mess. Thanks for the good UI update.
iterable 13y ago
This looks so much better
mortdeus 13y ago
Finally.
necubi 13y ago
This looks nice, but doesn't address my main complaint with the conversation/PR system: there's no way to batch comments. When doing code reviews, you can sometimes have upwards of 10 or 15 comments. In the current system this means 10 or 15 emails in everybody's inbox. It's also hard to work through a PR without letting everybody know when you're actually finished. I would love to be able to comment on things as I read through them, then after I finish publish all of the changes and have them go out as one email.
- Touche 13y ago
  I don't get any such emails. Maybe I turned it off years ago and forgot about it. Anyways, I wouldn't let an optional setting influence the way you work.
  - necubi 13y ago
    To be clear, I find the emails very useful (this is primarily from the perspective of me using Github for work--it's nice to know when somebody has reviewed your code). But I want one, not 15, and after the code reviewer has finished.
    - kisielk 13y ago
      I agree this is a big problem. I've had cases where I'm still reviewing stuff but the person whose code was under review just fixed my first few comments and then submitted it while I was still writing the rest. I know this is partially a process problem, but having batched emails would certainly help make it more obvious. Google's reitveld is a bit of a mess but at least it gets this part right.
    - shurcooL 13y ago
      Can't you turn off emails but still have GitHub notifications on the website?
      - rob 13y ago
        You can. I had to do this because my GitHub username (like my HN one) is just my name ("rob") and I was getting notifications from people using @rob in their comments/replies instead of the person's actual GitHub username. :)
    - Bockit 13y ago
      On the topic of github emails, I'd love the ability to set an email for notifications on a per repository, instead of just per organisation basis.
- Newky 13y ago
  Another advantage to this system is that reading and commenting becomes less prone to these sort of comments: "Where is X defined" "Ah I see, this is where X is defined, disregard older comment" A publish system allows you to basically stage comments before publishing. I used rietveld before github and this alongside side by side diffs was killer.
- JoshTriplett 13y ago
  Sounds like you want something closer to Gerrit, where you can do line-by-line code reviews on an entire commit along with an overall comment, and the whole thing goes out as one mail.
- welder 13y ago
  I found a :thumbsup: emoticon comment successfully tells everyone I'm finished reviewing a pull request.
  - kisielk 13y ago
    :+1: is a good shortcut :)
    - couchand 13y ago
      Is there a list of those somewhere? I've spent far too much time typing colon followed by random characters just to explore the many options.
      - icey 13y ago
        There sure is! http://www.emoji-cheat-sheet.com/ http://www.emoji-cheat-sheet.com/
- spicyj 13y ago
  Likewise. I find myself reluctant to comment on minor problems because I don't want to send another email. We use http://phabricator.org/ http://phabricator.org/ at work and I absolutely love it, but sometimes I'm forced to use GitHub for things and I get sad.
dj-wonk 13y ago
I'd like to be able to see a revision history when someone edits someone else's comment. Trust but verify. Sometimes people mess up.
- davis_m 13y ago
  I still don't understand why they allow people to edit other people's comments in the first place.
  - plorkyeran 13y ago
    It's very nice in the context of an open-source project to be able to fix formatting and such in people's bug reports, and to be able to update the original report with a more accurate summary of the problem when needed.
mitchelllc 13y ago
Do you guys think the headline in the example, " Upgrade to Normalize v3 #12406 Merged mdo merged 5 commits into master from normalize_v3 about 4 hours ago " occupy too many space in the web page?
Newky 13y ago
So I use github enterprise and use the review tool a lot. It has improved our overall code review experience, but there is a few things which have proved problematic and even have caused major headaches in the last year or so. 1. No Side by Side diffs. 2. Pushing new commits will often collapse conversations from the review. (i.e these conversations are folded) Really, I want some way to look at my PR and say hey I've addressed all the comments on this pull request, and I am good to merge. This is not even getting into what happens if you rebase your branch and then force a push to the remote.
- stu_k 13y ago
  For problem 1, I use this extension https://chrome.google.com/webstore/detail/octosplit/mnkacicafjlllhcedhhphhpapmdgjfbb https://chrome.google.com/webstore/detail/octosplit/mnkacica... It's not quite the side-by-side diff that I want (like Meld http://meldmerge.org/ http://meldmerge.org/) but it does help.
- jleader 13y ago
  When things are messy enough that I want to rebase a branch, I generally create a new branch and new PR, and close the old PR with a comment linking to the new PR. I also put a link to the old PR in the description of the new one, so people can easily go back and look at the old discussion. In other words, rebasing is a big enough change that I feel it's appropriate to restart the PR.
  - taeric 13y ago
    If you are linking the requests with a comment on each end, I'm not sure I see the benefit to just pushing the rebase into an open request. Is there a real difference?
- spicyj 13y ago
  At Khan Academy we use http://phabricator.org/ http://phabricator.org/ and I absolutely love it. It solves both of your named problems and a handful of others.
- sarm2005 13y ago
Glyptodon 13y ago
I'm seeing a lot of overlapping text and box breaking.
shurcooL 13y ago
Am I the only one who dislikes that they made all the fonts slightly larger? I'm working on a 1080p monitor atm and I really don't want more screen real estate taken up with no benefit.
- guynamedloren 13y ago
  If this is your only complaint, I think it's safe to say that they're doing pretty well overall.
  - shurcooL 13y ago
    Oh, absolutely, I love everything else about the change. It makes the conversation and commits/etc. much easier to follow.
- rhizome 13y ago
  I wonder why in the age of responsive design that on-page or user-settings oriented fontsize adjustment wouldn't be more prevalent.
  - shurcooL 13y ago
    Or the ability to have consistent tab settings that match your preference. https://github.com/isaacs/github/issues/57 https://github.com/isaacs/github/issues/57
- chrismorgan 13y ago
  I like the larger fonts.
  - shurcooL 13y ago
    Interesting. I'm curious, can you share your monitor(s) setup (i.e., size, resolution, DPI scaling if any, OS, etc.) and the type of work you do please?
    - 13y ago
obsession 13y ago
This is a good redesign but I wish they put more focus on developing Github Issues. The back button doesn't work (old issues randomly show) and filtering needs work (for example, how can I show issues from two milestones? how about showing issues with no comments? etc.).
trustfundbaby 13y ago
The other thing I'd like to see is context on emails in a conversation thread. So if I reply to a comment you've made, it would be great to see the comment I'm replying to IN the email itself (or at the very least, the last comment on that line, if there are multiple comments). A lot of times I'll get an email comment and have no idea what they're talking about till I click through to go back to the conversation thread
- reledi 13y ago
  Please do others a favour and don't reply to GitHub comments from email! Many people, including myself, will edit a comment a few times after publishing it. So the comment you're replying to by email would be outdated. I've gone so far to disable email notifications completely. Which is another win for me because I can't stand being blasted with emails all the time. It's like a never ending to do list which builds up extremely quickly.
kimonos 13y ago
I like this one!
coherentpony 13y ago
And replies still aren't nested? Guys, multi-party conversations are not necessarily linear.
erichurkman 13y ago
Thank you for making the issue number large enough to read where I don't have to hunt and squint to find it!
Toddward 13y ago
Overall, I like what I'm seeing so far. The notable exception is that they removed the build status indicator for Pull Requests from the PR body and now only show the faint green check next to a commit hash. Makes determining whether a PR is good to merge a bit more difficult IMHO. Hopefully that gets fixed.

126 points by bencevans 13y ago

urbanachiever 13y ago

Brings a lot of clarity to what used to be a mess. Thanks for the good UI update.

iterable 13y ago

This looks so much better

mortdeus 13y ago

Finally.

necubi 13y ago

This looks nice, but doesn't address my main complaint with the conversation/PR system: there's no way to batch comments. When doing code reviews, you can sometimes have upwards of 10 or 15 comments. In the current system this means 10 or 15 emails in everybody's inbox. It's also hard to work through a PR without letting everybody know when you're actually finished. I would love to be able to comment on things as I read through them, then after I finish publish all of the changes and have them go out as one email.

Touche 13y ago
I don't get any such emails. Maybe I turned it off years ago and forgot about it. Anyways, I wouldn't let an optional setting influence the way you work.
- necubi 13y ago
  To be clear, I find the emails very useful (this is primarily from the perspective of me using Github for work--it's nice to know when somebody has reviewed your code). But I want one, not 15, and after the code reviewer has finished.
  - kisielk 13y ago
    I agree this is a big problem. I've had cases where I'm still reviewing stuff but the person whose code was under review just fixed my first few comments and then submitted it while I was still writing the rest. I know this is partially a process problem, but having batched emails would certainly help make it more obvious. Google's reitveld is a bit of a mess but at least it gets this part right.
  - shurcooL 13y ago
    Can't you turn off emails but still have GitHub notifications on the website?
    - rob 13y ago
      You can. I had to do this because my GitHub username (like my HN one) is just my name ("rob") and I was getting notifications from people using @rob in their comments/replies instead of the person's actual GitHub username. :)
  - Bockit 13y ago
    On the topic of github emails, I'd love the ability to set an email for notifications on a per repository, instead of just per organisation basis.
Newky 13y ago
Another advantage to this system is that reading and commenting becomes less prone to these sort of comments: "Where is X defined" "Ah I see, this is where X is defined, disregard older comment" A publish system allows you to basically stage comments before publishing. I used rietveld before github and this alongside side by side diffs was killer.
JoshTriplett 13y ago
Sounds like you want something closer to Gerrit, where you can do line-by-line code reviews on an entire commit along with an overall comment, and the whole thing goes out as one mail.
welder 13y ago
I found a :thumbsup: emoticon comment successfully tells everyone I'm finished reviewing a pull request.
- kisielk 13y ago
  :+1: is a good shortcut :)
  - couchand 13y ago
    Is there a list of those somewhere? I've spent far too much time typing colon followed by random characters just to explore the many options.
    - icey 13y ago
      There sure is! http://www.emoji-cheat-sheet.com/ http://www.emoji-cheat-sheet.com/
spicyj 13y ago
Likewise. I find myself reluctant to comment on minor problems because I don't want to send another email. We use http://phabricator.org/ http://phabricator.org/ at work and I absolutely love it, but sometimes I'm forced to use GitHub for things and I get sad.

dj-wonk 13y ago

I'd like to be able to see a revision history when someone edits someone else's comment. Trust but verify. Sometimes people mess up.

davis_m 13y ago
I still don't understand why they allow people to edit other people's comments in the first place.
- plorkyeran 13y ago
  It's very nice in the context of an open-source project to be able to fix formatting and such in people's bug reports, and to be able to update the original report with a more accurate summary of the problem when needed.

mitchelllc 13y ago

Do you guys think the headline in the example, " Upgrade to Normalize v3 #12406 Merged mdo merged 5 commits into master from normalize_v3 about 4 hours ago " occupy too many space in the web page?

Newky 13y ago

So I use github enterprise and use the review tool a lot. It has improved our overall code review experience, but there is a few things which have proved problematic and even have caused major headaches in the last year or so. 1. No Side by Side diffs. 2. Pushing new commits will often collapse conversations from the review. (i.e these conversations are folded) Really, I want some way to look at my PR and say hey I've addressed all the comments on this pull request, and I am good to merge. This is not even getting into what happens if you rebase your branch and then force a push to the remote.

stu_k 13y ago
For problem 1, I use this extension https://chrome.google.com/webstore/detail/octosplit/mnkacicafjlllhcedhhphhpapmdgjfbb https://chrome.google.com/webstore/detail/octosplit/mnkacica... It's not quite the side-by-side diff that I want (like Meld http://meldmerge.org/ http://meldmerge.org/) but it does help.
jleader 13y ago
When things are messy enough that I want to rebase a branch, I generally create a new branch and new PR, and close the old PR with a comment linking to the new PR. I also put a link to the old PR in the description of the new one, so people can easily go back and look at the old discussion. In other words, rebasing is a big enough change that I feel it's appropriate to restart the PR.
- taeric 13y ago
  If you are linking the requests with a comment on each end, I'm not sure I see the benefit to just pushing the rebase into an open request. Is there a real difference?
spicyj 13y ago
At Khan Academy we use http://phabricator.org/ http://phabricator.org/ and I absolutely love it. It solves both of your named problems and a handful of others.
sarm2005 13y ago

Glyptodon 13y ago

I'm seeing a lot of overlapping text and box breaking.

shurcooL 13y ago

Am I the only one who dislikes that they made all the fonts slightly larger? I'm working on a 1080p monitor atm and I really don't want more screen real estate taken up with no benefit.

guynamedloren 13y ago
If this is your only complaint, I think it's safe to say that they're doing pretty well overall.
- shurcooL 13y ago
  Oh, absolutely, I love everything else about the change. It makes the conversation and commits/etc. much easier to follow.
rhizome 13y ago
I wonder why in the age of responsive design that on-page or user-settings oriented fontsize adjustment wouldn't be more prevalent.
- shurcooL 13y ago
  Or the ability to have consistent tab settings that match your preference. https://github.com/isaacs/github/issues/57 https://github.com/isaacs/github/issues/57
chrismorgan 13y ago
I like the larger fonts.
- shurcooL 13y ago
  Interesting. I'm curious, can you share your monitor(s) setup (i.e., size, resolution, DPI scaling if any, OS, etc.) and the type of work you do please?
  - 13y ago

obsession 13y ago

This is a good redesign but I wish they put more focus on developing Github Issues. The back button doesn't work (old issues randomly show) and filtering needs work (for example, how can I show issues from two milestones? how about showing issues with no comments? etc.).

trustfundbaby 13y ago

The other thing I'd like to see is context on emails in a conversation thread. So if I reply to a comment you've made, it would be great to see the comment I'm replying to IN the email itself (or at the very least, the last comment on that line, if there are multiple comments). A lot of times I'll get an email comment and have no idea what they're talking about till I click through to go back to the conversation thread

reledi 13y ago
Please do others a favour and don't reply to GitHub comments from email! Many people, including myself, will edit a comment a few times after publishing it. So the comment you're replying to by email would be outdated. I've gone so far to disable email notifications completely. Which is another win for me because I can't stand being blasted with emails all the time. It's like a never ending to do list which builds up extremely quickly.

kimonos 13y ago

I like this one!

coherentpony 13y ago

And replies still aren't nested? Guys, multi-party conversations are not necessarily linear.

erichurkman 13y ago

Thank you for making the issue number large enough to read where I don't have to hunt and squint to find it!

Toddward 13y ago

Overall, I like what I'm seeing so far. The notable exception is that they removed the build status indicator for Pull Requests from the PR body and now only show the faint green check next to a commit hash. Makes determining whether a PR is good to merge a bit more difficult IMHO. Hopefully that gets fixed.