undefined | Better HN

0 points__david__11y ago0 comments

> One thing that git stinks at is line-by-line code review, for example.

Really? That's one thing I haven't ever seen anyone do better than plain old 'git log -p'. I use this alias:

  [alias]
      review = log -p --reverse

After `git fetch`-ing I copy the hash range that I fetched and paste it after `git review`. Then in Less I can type "/^diff" to use n and p to skip back and forth between commits. I find this is much easier to read than Github's html diff views and infinitely more efficient than the millions of clicks I need in web browser to do anything (plus there's absolutely no lag, unlike the browser).

0 comments

lomnakkus11y ago

How are you gathering up comments and making sure that all issues are addressed, etc.?

__david__OP11y ago

By talking to the person, in person or on the phone or IM or email, depending on the priority of the question/issue. If something needs to be addressed then it goes into the bug tracker (whatever that might be). For egregious things, we might even `git revert` the patch and let the person rethink/rework it.

lomnakkus11y ago

That doesn't address the actual recording of the comments such that they can easily see the context of your comments. It also doesn't adress pre-merge review and pre-merge integration automated testing.

In short: No. Although I'm happy that your process works for you it doesn't (and won't) work for a lot of other situations (including mine at my current place of work nor my previous one).

__david__OP11y ago

Recording of comments is pretty pointless. The code is what matters and that is safely in git. Anything really important should go in the commit message, where it belongs.

If the comments are that important to you then you are foolish to use Github's interface because that means that you can only ever use Github to look at the patch and see the comments. And there's never an indication that the patch even has any comments on it in the git repo itself (rendering git log useless).

It would be much better to use Bugzilla (or something similar) the way Mozilla does, with bug numbers referenced in the commits and commits referenced in the bugs.

There's also "git notes" but it has some funky corner cases IIRC.

I also am not understanding what Github's clunky commit viewer has to do with "pre-merge integration automated testing". That sounds like CI territory.

erikb11y ago

I don't get your problem with automated testing. In my eyes this is the one feature that the shell has, but Github doesn't. That's why I'm quite confused. You can addd git hooks to automate it, or simply call the corresponding command while you are in the shell already. But GitHub neither builds nor tests your code, it simply shows you the git log, git diff, etc. as HTML.

1 more reply

j / k navigate · click thread line to collapse

0 comments

lomnakkus11y ago

How are you gathering up comments and making sure that all issues are addressed, etc.?

__david__OP11y ago

lomnakkus11y ago

In short: No. Although I'm happy that your process works for you it doesn't (and won't) work for a lot of other situations (including mine at my current place of work nor my previous one).

__david__OP11y ago

Recording of comments is pretty pointless. The code is what matters and that is safely in git. Anything really important should go in the commit message, where it belongs.

It would be much better to use Bugzilla (or something similar) the way Mozilla does, with bug numbers referenced in the commits and commits referenced in the bugs.

There's also "git notes" but it has some funky corner cases IIRC.

I also am not understanding what Github's clunky commit viewer has to do with "pre-merge integration automated testing". That sounds like CI territory.

erikb11y ago

1 more reply

j / k navigate · click thread line to collapse