4 ms·

Github PRs (at least on the enterprise version) still have the problem of collapsing comments where the associated line of code changed in some way that had not

by u801e 6y ago

Github PRs (at least on the enterprise version) still have the problem of collapsing comments where the associated line of code changed in some way that had nothing to do with the comment. They also require a lot of scrolling if the overall diff is rather long. And, they still don't provide a way to comment on commit messages themselves.

raverbashing 6y ago
Agreed on the comment collapse, about comment on the commit message I'd say it's a commit on the main PR (since by the time the PR is done the commit msgs are pretty much not changeable, except for the last one)
- u801e 6y ago
  > about comment on the commit message I'd say it's a commit on the main PR Except that if the overall diff line I choose to comment on about the commit message changes in some way, then the comment is collapsed, which makes it difficult to keep track of what needs to change. > since by the time the PR is done the commit msgs are pretty much not changeable, except for the last one They can be changed via a git rebase.
plorkyeran 6y ago
Collapsing comments when the code changes is the correct thing to do 90% of the time in my experience, and for that other 10% you can manually mark is as unresolved. It's the correct default IMO.
- u801e 6y ago
  > Collapsing comments when the code changes is the correct thing to do 90% of the time in my experience Why not just leave them uncollapsed and allow one to mark them as resolved when the change is actually resolved. The other problem is that it's difficult to see what actually changed when a line is marked out of date without having to scan through the entire updated diff and then jump back and forth between the diff and conversation view. Collapsing them by default makes it difficult to find the comment by scanning. If they were uncollapsed until I marked them as resolved, then I could at least use the find feature in the browser and search for my username to find my comments.

by u801e 6y ago

raverbashing 6y ago

Agreed on the comment collapse, about comment on the commit message I'd say it's a commit on the main PR (since by the time the PR is done the commit msgs are pretty much not changeable, except for the last one)

u801e 6y ago
> about comment on the commit message I'd say it's a commit on the main PR Except that if the overall diff line I choose to comment on about the commit message changes in some way, then the comment is collapsed, which makes it difficult to keep track of what needs to change. > since by the time the PR is done the commit msgs are pretty much not changeable, except for the last one They can be changed via a git rebase.

plorkyeran 6y ago

Collapsing comments when the code changes is the correct thing to do 90% of the time in my experience, and for that other 10% you can manually mark is as unresolved. It's the correct default IMO.

u801e 6y ago
> Collapsing comments when the code changes is the correct thing to do 90% of the time in my experience Why not just leave them uncollapsed and allow one to mark them as resolved when the change is actually resolved. The other problem is that it's difficult to see what actually changed when a line is marked out of date without having to scan through the entire updated diff and then jump back and forth between the diff and conversation view. Collapsing them by default makes it difficult to find the comment by scanning. If they were uncollapsed until I marked them as resolved, then I could at least use the find feature in the browser and search for my username to find my comments.