Verified Commit 7bb4f190 authored by doshitan's avatar doshitan
Browse files

Minor updates to code-review/ and git/

parent 6823461d
......@@ -2,7 +2,7 @@
title: Code Review
toc: true
published: 2019-07-28T22:48:15-05:00
modified: 2020-09-24T22:19:41-04:00
modified: 2020-10-14T22:50:25-04:00
---
For me, code review is usually the biggest -- and often most useful --
......@@ -11,11 +11,11 @@ rubber actually hitting the road in development work. It can be highly
collaborative and helpful.
Like anything, it can be poorly utilized. And it probably shouldn't be the
*first* touch point for collaboration[fn::There are times when code changes are
the first touch point, they may be the clearest and easiest way to see
something, but often this is not be the case.]; design documents, pairing, etc.
are great ways to share knowledge and build good stuff. But ultimately, changes
to the system have to actually become realized, and that happens through code.
*first* touch point for collaboration[fn::There are times when code changes may
be the clearest and easiest way to think about something, but often this is not
the case.]; design documents, pairing, etc. are great ways to share knowledge
and build good stuff. But ultimately, changes to the system have to actually
become realized, and that happens through code.
In a professional context, a team should generally prioritize code review.
Everyone should try to get to new requests or feedback as quickly as possible
......@@ -69,11 +69,12 @@ of the changes that can't capture adequately in the tests.
testing your changes.
Write [[https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code/][reviewable code]]. Generally try to keep commit/change sizes small and
focused on one thing. If you find yourself using words like "and" or "also" in
your title that might mean the changes could be broken down better[fn::This is
something I am continuously working at getting better at. So often I'll find
little things that I fix in the course of some other work that I really should
break out separately, but just include because I'm lazy. Do your best.].
focused on one thing. The [[https://secure.phabricator.com/book/phabflavor/article/recommendations_on_revision_control/][one idea is one commit]] approach may be worth
considering. If you find yourself using words like "and" or "also" in your title
that might mean the changes could be broken down better[fn::This is something I
am continuously working at getting better at. So often I'll find little things
that I fix in the course of some other work that I really should break out
separately, but just include because I'm lazy. Do your best.].
Take feedback graciously (which has hopefully been given kindly).
......@@ -127,7 +128,7 @@ Things to keep in mind while providing feedback:
And I'll reiterate, make your intentions clear with feedback. If something is a
suggestion, make sure it's obviously so. If something is a blocker, state that.
If you want a system to follow: https://conventionalcomments.org/
If you want a system to follow: [[https://conventionalcomments.org/][conventionalcomments.org]]
After going over the changes in detail, read over the entire
module/file/document with the changes. Don't get tunnel vision looking only at
......@@ -167,4 +168,4 @@ understandable is good.
* Misc.
- https://google.github.io/eng-practices/review/
- https://thoughtbot.com/blog/five-tips-for-more-helpful-code-reviews
- http://mth.io/posts/code-review/
- https://mth.io/posts/code-review/
......@@ -3,14 +3,14 @@ title: Git Tips
toc: true
toc-margin: true
published: 2019-04-16T11:23:06-05:00
modified: 2019-08-21T08:26:51-04:00
modified: 2020-10-14T22:50:25-04:00
---
I love git, such an good tool. It's the only VCS I've ever used in anger and is
so ubiquitous now, hard to see seriously using something else^[Though I think
theoretically I prefer something like [darcs](http://darcs.net/), I'm usually
pushed back to git. I'm hopeful for [Pijul](https://pijul.org/) to improve upon
darcs.].
I love git, it's such a good tool. Granted, it's the only VCS I've ever used in
anger, but is so ubiquitous now, hard to see seriously using something
else^[Though I think theoretically I prefer something like
[darcs](http://darcs.net/), I'm usually pushed back to git. I'm hopeful for
[Pijul](https://pijul.org/) to improve upon darcs.].
It also has a fun birth story, and in 2016 an added twist of irony.
[Seriously](https://git-scm.com/book/en/v2/Getting-Started-A-Short-History-of-Git),
......@@ -50,9 +50,9 @@ usage of `reset` and `checkout` commands as an illustration. I'll give a short
description of the mental model here.
First thing to note is that the git repository really lives in the `.git/`
directory^[This is the content of a "bare" repository, if you `git init --bare`,
which creates a repo without a work tree.]. This is the full content, all
history and files, stored in git's internal data format.
directory^[This is the content of a "bare" repository, such as if you `git init
--bare`, which creates a repo without a work tree.]. This is the full content,
all history and files, stored in git's internal data format.
The first two trees live there.
......@@ -64,7 +64,7 @@ The Index tree, points to the next commit. This is the staging area, what you
The "repo" that you work in day-to-day, the actual files you edit are called the
"working directory" or work tree. When you change branches or anything, git
resolves it's internal data for the given commit into real files in the work
tree so you can, well, work with them easier.
tree so you can, well, work with them.
Most of the commands you use regularly are around manipulating the state of
these three trees. Adding changes from the work tree into the index, then
......@@ -75,6 +75,17 @@ tree based on the new `HEAD`.
For more, read through the [full
article](https://git-scm.com/book/en/v2/Git-Tools-Reset-Demystified).
You may also find this [interactive
example](https://ndpsoftware.com/git-cheatsheet.html) helpful, though it uses
slightly different terminology:
- `HEAD` tree → Local Repository
- Index tree → Index (this one is the same)
- Work tree → Workspace
And it also shows how some commands interact with the stash and remote copy of
the repository, going a little beyond the "core" three trees.
# pathspec
A less commonly learned, but useful bit of info to read and have as references
......@@ -137,34 +148,34 @@ messages](https://secure.phabricator.com/book/phabflavor/article/writing_reviewa
# Rebase/rewriting history
I rebase a lot in my usual [workflow](#my-preferred-workflow). I rewrite history
a lot, sqaushing commits together, breaking them apart. Some folks are scared of
this. Others just hate it, there is an age old [philosophical
I rebase a lot in my usual [workflow](#my-preferred-workflow), rewriting
history, sqaushing commits together, breaking them apart, etc. Some folks are
scared of this. Others hate it. There is an age old [philosophical
divide](https://git-scm.com/book/en/v2/Git-Branching-Rebasing#_rebase_vs_merge).
It's clear where I fall and not going to get into it here, just give some tips
on how I tend to use this stuff.
First, "rebase". Your work, as represented by a sequence of commits, come after
another commit, they are "based" on that commit, stem from it, so to "rebase",
means to choose a new "base" for your work, literally pick up a sequence of
commits and move them on top of a different starting commit.
First, "rebase", the word itself. Your work, as represented by a sequence of
commits, come after another commit, they are "based" on that commit, stem from
it; so to "re-base", means to choose a new "base" for your work, literally pick
up a sequence of commits and move them on top of a different starting commit.
When you just want to rebase on branch on another that has some new commits, a
plain `git rebase <branch>` is usually enough. The `--onto` flag to `git rebase`
comes into play in more complicated situations or when you just want to be
explicit.
When you want to rebase a branch on another that has some new commits, a plain
`git rebase <branch>` can be enough in simple situation. The `--onto` flag to
`git rebase` comes into play in more complicated situations or when you want to
be explicit.
A simplified look at it, in the form of `git rebase --onto <branch> <commit
before the first one you want>`. Think of it as "pick up all commits after
<commit> and apply them on top of (i.e., onto) <branch>."
`<commit>` and apply them on top of (i.e., onto) `<branch>`."
This comes into play a bunch when you are working on a feature branch (`feat-b`)
that depends on another feature branch (`feat-a`). As the other feature branch
continues development, maybe rebases on `master`, you want to catch your branch
up, just a `git rebase feat-a` from `feat-b` is likely to get confused. A `git
rebase --onto feat-a <commit before your work in feat-b>` can get it sorted.
Let's look at an example from the rebase help page:
that depends on another feature branch (`feat-a`). As `feat-a` continues
development, maybe itself rebases on `master`, and you want to catch your branch
up, just a `git rebase feat-a` from `feat-b` will likely not do what you want. A
`git rebase --onto feat-a <commit before your work in feat-b>` can get it
sorted. Let's look at an example from the rebase help page:
```
First let’s assume your topic is based on branch next. For example, a feature developed in topic depends on some functionality which is found in next.
......@@ -196,9 +207,9 @@ them out all together.
Let's say you have some work on a branch `new-wizbang-feature` and you want to
clean up it's history. You can run `git rebase -i master` on the branch, this
will bring up your editor with a list of commits with the work `pick` in front
of each. You options will be explained in the buffer as well, but you can change
that work `pick` to any of the other options to take that action on a commit.
will bring up your editor with a list of commits with `pick` in front of each.
You options will be explained in the buffer as well, but you can change that
`pick` to any of the other options to take that action on a commit.
When you stop to perform some action on a commit, issue `git rebase --continue`
(my alias `git rc`) to move on to the next action. If you ever get confused or a
......@@ -221,12 +232,12 @@ version of the thing.
Development happens in feature branches, when the feature is ready, it is
rebased and merged to master. I like a clean linear history, but if you like
merge commits, that's fine just make the merge commit message useful so when
viewing the history with `--first-parent` you can easily track what is actually
happening to the codebase.
merge commits, that's fine, I only suggest making the merge commit message
useful so when viewing the history with `--first-parent` you can easily track
what is actually happening to the codebase.
Note that when I say "feature branch" I just mean a branch the changes to
support the feature lives while being developed and reviewed. Controlling the
Note that when I say "feature branch" I just mean a branch where the changes to
support the feature live while being developed and reviewed. Controlling the
"feature" in production should be done with feature flags on the application,
that is, the code is merged to `master` and there is a run-time system to enable
or disable the new code for testing/validation/experimentation. The "feature
......@@ -235,11 +246,11 @@ mechanism.
Looking at the default GitHub template for merge commit messages for example,
`Merge pull request #xxx from <user>/<branch>`, isn't the worst if the branch
name is reasonable, but the information I most care about, what does this commit
do in terms of the code is pushed to the end of the line after a bunch of
gobbledygook. When looking through the history, it would be nicer to see `Do
thing` with the `Merge pull ...` details in the summary section, easily
available, but out of the way.
name is reasonable, but even so, the information I most care about, what this
commit *does*, is pushed to the end of the line after a bunch of gobbledygook.
When looking through the history, it would be nicer to see `Do thing` with the
`Merge pull ...` details in the summary section, easily available, but out of
the way.
And I'm against using merges as an excuse to let a messy, meandering history
into the master branch. Some folks like including every part of the dev process
......@@ -258,7 +269,7 @@ system with the full conversation and iteration the code went through.
So if you must do merges, instead of:
```
* Do thing
* Merge some stuff from Joan
|\
| * Fix
| * Oops, redo
......@@ -270,7 +281,7 @@ So if you must do merges, instead of:
| * Start thing
|/
* Another thing
* New feature
* Merge other stuff from Bob
|\
| * Feedback fixes
| * Try this other thing
......@@ -313,10 +324,12 @@ the [one idea is one
commit](https://secure.phabricator.com/book/phabflavor/article/recommendations_on_revision_control/)
approach.
Versions get tagged, if fixes or features need backported, branch from
the tag, apply necessary changes and tag new release on that branch. If you
don't do "versions"/releases, but just regularly want to promote work to some
"stable" state, then have a `stable` branch that you update when appropriate.
Versions get tagged[^With an [annotated
tag](https://git-scm.com/book/en/v2/Git-Basics-Tagging#_creating_tags), not a
lightweight one.], if fixes or features need backported, branch from the tag,
apply necessary changes and tag new release on that branch. If you don't do
"versions"/releases, but just regularly want to promote work to some "stable"
state, then have a `stable` branch that you update when appropriate.
Every commit that hits `master` should be "buildable"^[The definition of which
could vary based on the project.]. For the rare cases where there is some large
......@@ -357,8 +370,8 @@ Or possibly your test suite takes a long time to run, you can kick it off on a
codebase in a separate worktree while continuing work on a different on in the
mean time. Similar for compiling a big thing.
Or you just need to switch between some branches a bunch and are tired of
worrying about stashing changes every time you switch.
Or you just need to switch between some branches a bunch and tire of worrying
about stashing changes every time you switch.
This is supported by git with
[worktrees](https://git-scm.com/docs/git-worktree). This allows you checkout
......@@ -366,7 +379,7 @@ more than one branch from a single git repository.
You can of course do this by cloning the repo multiple times (or copying `.git/`
to a new dir, same thing), which is fine, but this has a couple downsides. One,
it multiples the amount of disk space taken, for large repos this could be an
it multiplies the amount of disk space used, for large repos this could be an
issue. Two, it adds another layer to manage, you now need to pull updates for
each of the clones, which is inconvenient.
......@@ -456,10 +469,16 @@ git checkout <branch> -- <file path>
# Misc. Stuff
- <http://ndpsoftware.com/git-cheatsheet.html>
- [Knowledge is Power: Getting out of trouble by understanding Git by Steve
Smith ](https://www.youtube.com/watch?v=sevc6668cQ0) (basic intro)
- <http://h2.jaguarpaw.co.uk/posts/git-survival-guide/> (very opinionated use of git)
- When you need to nuke things: <https://stackoverflow.com/a/64966>
- When you need to fix things: <https://sethrobertson.github.io/GitFixUm/fixup.html>
- <https://www.makeareadme.com/>
- [ndpsoftware.com/git-cheatsheet.html](https://ndpsoftware.com/git-cheatsheet.html)
- [Knowledge is Power: Getting out of trouble by understanding
Git](https://www.youtube.com/watch?v=sevc6668cQ0) by Steve Smith (basic intro)
- [h2.jaguarpaw.co.uk/posts/git-survival-guide/](http://h2.jaguarpaw.co.uk/posts/git-survival-guide/)
(very opinionated use of git)
- When you need to nuke things:
[stackoverflow.com/a/64966](https://stackoverflow.com/a/64966)
- When you need to fix things:
[sethrobertson.github.io/GitFixUm/fixup.html](https://sethrobertson.github.io/GitFixUm/fixup.html)
- [makeareadme.com](https://www.makeareadme.com/)
- [Git Koans by Steve Losh](https://stevelosh.com/blog/2013/04/git-koans/)
- [legends2k.github.io/note/git_concepts/](https://legends2k.github.io/note/git_concepts/)
- [legends2k.github.io/note/git_nuances/](https://legends2k.github.io/note/git_nuances/)
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment