4 ms·

Assuming that we're talking about the same thing, this is possible in GH from the `Files` tab on the PR screen.

u801e 6y ago
Not really. You cannot comment on the commit message itself. And, if I'm not mistaken, changes made in one commit that are undone in a subsequent commit won't show up even if the files view if filtered by commit. Also, it's difficult to tell which comment is associated with which commit.
- conradludgate 6y ago
  I don't think I ever factor in commit messages into my code reviews. I may look at the commit history if I want to see how they experimented implementing the feature, but I only care about the end goal and the code I will be merging in at the end.
  - u801e 6y ago
    The issue is that Github actually discourages reviewing by commit because it makes it difficult to do so. Though there are those who are only interested in the overall diff, there are others who want to be able to maintain a clean and informative history via version control. When the review system makes it difficult to ensure that's the case, then then version control history cannot be maintained.
    - atq2119 6y ago
      Precisely. We're in a vicious cycle where most people using Git don't use what ought to be Git best practices for most serious large scale projects (clean history with self-contained logical commits and good commit descriptions) because GitHub makes it so difficult to do this. And to be fair, GitLab really isn't much better in this department. And there isn't enough pressure on GitHub or GitLab to clean up their act because most people don't even know what they're missing. And so the cycle continues.
      - felipelemos 6y ago
        If you use squash and merge this won't be a problem, you will have a "clean history with self-contained logical commits and good commit history", and intermediary commits won't matter anyway.
        u801e 6y ago
        Some features take more than a single logical commit to implement. Looking at the git mailing list, I see patch series containing anywhere from 1 to 10 commits (with some as many as 15 commits). Also, having a single commit touching many parts of the code makes it harder to revert due to a greater chanche of conflicts with other commits.

Not really. You cannot comment on the commit message itself. And, if I'm not mistaken, changes made in one commit that are undone in a subsequent commit won't show up even if the files view if filtered by commit. Also, it's difficult to tell which comment is associated with which commit.

conradludgate 6y ago
I don't think I ever factor in commit messages into my code reviews. I may look at the commit history if I want to see how they experimented implementing the feature, but I only care about the end goal and the code I will be merging in at the end.
- u801e 6y ago
  The issue is that Github actually discourages reviewing by commit because it makes it difficult to do so. Though there are those who are only interested in the overall diff, there are others who want to be able to maintain a clean and informative history via version control. When the review system makes it difficult to ensure that's the case, then then version control history cannot be maintained.
  - atq2119 6y ago
    Precisely. We're in a vicious cycle where most people using Git don't use what ought to be Git best practices for most serious large scale projects (clean history with self-contained logical commits and good commit descriptions) because GitHub makes it so difficult to do this. And to be fair, GitLab really isn't much better in this department. And there isn't enough pressure on GitHub or GitLab to clean up their act because most people don't even know what they're missing. And so the cycle continues.
    - felipelemos 6y ago
      If you use squash and merge this won't be a problem, you will have a "clean history with self-contained logical commits and good commit history", and intermediary commits won't matter anyway.
      - u801e 6y ago
        Some features take more than a single logical commit to implement. Looking at the git mailing list, I see patch series containing anywhere from 1 to 10 commits (with some as many as 15 commits). Also, having a single commit touching many parts of the code makes it harder to revert due to a greater chanche of conflicts with other commits.