| Age | Commit message (Collapse) | Author |
|---|
|
True, the remote name is not necessarily going to be called "origin",
but that's a reasonable default.
This allows commit ranges to work even when the merge base branch hasn't
been created locally.
|
|
|
|
It's more convenient to have the "..." here because all of the
code-review commands use `get_merge_base`. Otherwise I'd end up having
to add it to most of the other commands.
But now, instead of only adding it to the default merge base, add it to
all merge bases, including the one set by `code-review-start`. This
makes all merge bases consistent, and means you don't have to include
the dots when setting a custom merge base.
|
|
This is inspired by a Git alias I have, `git ddiff`, that opens the Git
diff in the Delta diff viewer (https://github.com/dandavison/delta).
Delta ends up being much nicer than regular Git diff for code reviews
because it renders syntax highlighting.
|
|
I was inspired by Jake Zimmerman's blog post "Code Review from the
Command Line" (https://blog.jez.io/cli-code-review/) to add an
environment variable override for the merge base. This allows us to set
a different merge base for individual command executions.
|
|
This allows us to pass along arguments like `--ignore-all-space`.
|
|
Previously, the focused window in each tab was the Fugitive window that
contained the old version of the file. It's nicer to have the new
version focused by default, because that allows you to make changes to
the local source code.
|
|
I don't think this is the right approach, but since I added the "..." in
`code-review-changed-files` in 7659cf07f118fec4c1ba9677c82a7ef61511e797,
removing the "..." from here allows the difftool script to work when
using the default merge base.
|
|
Doesn't cover all the cases, but I think this should give us the correct
set of files when the merge base has been set via `code-review-start`.
Otherwise we might end up with a bunch of files that came from other
branches, not the one we're reviewing now.
|
|
|
|
|
|
Realistically I should match based on the executable files available
that start with "code-review-", but this is how it works currently, so
it's less effort to add to what's already here.
|
|
Haven't tested this yet, but the idea is to use the `gh` CLI to call the
GitHub API and get the list of pulls requests. These are filtered to get
the one open for the current branch, and we can then get its merge base
and start a `code-review` review using that merge base.
|
|
|
|
To alleviate dealing with repositories that don't use "master" as their
master branch.
|
|
The Fugitive documentation says that `Gdiff` is deprecated. In the
current version, you need to use the `Gvdiffsplit` command to get a
side-by-side diff with a vertical split.
|
|
|
|
Wraps `git log` to show commits since the merge base.
|
|
When the files being opened with `difftool` are already open in a Vim
instance, a 'swap file already exists' confirmation warning is displayed
for each file. Bypass the warning by not saving swap files (using the -n
command line option).
|
|
Turns out I don't need to worry about setting or unsetting `extglob` at
all because the script gets executed in a subshell. Obviously. Didn't
realise or test thoroughly at first.
|
|
If `extglob` was set, we'd always unset it. Check to see if it's set
before doing anything with it, and then restore it to its original
value.
|
|
This was a note preceding implementation, and is no longer needed.
|
|
Add the range filter to try to remove commits that we don't care about.
Still not sure if this will work in all cases, as sometimes `..` seemed
to give me the right collection of files and sometimes `...` did.
code-review-difftool:
Pass the merge base without range dots (`..` or `...`) to Vim Fugitive,
as it doesn't accept revisions with ranges. Not sure what to do if I get
a range with two revision names yet, though.
Turns on Bash's `extglob` in order to use `?(...)` to include both
patterns in the expansion.
|
|
If the database doesn't exist, we get this error:
Error: near line 3: no such table: merge_bases
This is because `sqlite3` will always create a new database if it
doesn't exist when running a command against a database. The
newly-created database doesn't have the required table, and we get the
error.
Fix the problem by ensuring the database always exists and is
initialised properly.
|
|
This was taken care of in 21534162124501f0410ee31cd31130aa3b92a9f3.
|
|
|
|
The `shift` call came before I was checking the first argument `$1` if
it was '--help'. This caused the program to print the 'nonexistent
command' error message. Move the `shift` call to just before we pass on
the rest of the command line arguments.
|
|
|
|
|
|
Not very detailed at the moment, just copies the usage output. Would be
nice to make this more descriptive, and even better to have separate man
pages for each subcommand.
|
|
Print a real usage message.
|
|
|
|
Use the existing `code-review` tool instead of the full Git command.
|
|
Basic Bash completion of `code-review` subcommands.
|
|
* Use the common merge base from `code-review-database`.
* Print an absolute path to the file using `$GIT_ROOT` from
`code-review-database`. This allows the paths to be useful even when
PWD is a subdirectory in the Git repo.
|
|
Make the script a `code-review` subcommand. I had originally written it
before the `code-review-` subcommand prefix system.
|
|
Replace `$ARGS` system from before with the merge base value taken from
the database. This makes the merge base consistent across `code-review`
tools/subcommands.
|
|
Don't duplicate rows if we call `create_merge_base` multiple times.
Doing so would mean we'd get multiple merge base rows for the same head.
Instead, overwrite the existing merge base for a given head using an
upsert.
Thanks a bunch to MarqueIV
(https://stackoverflow.com/users/168179/marqueiv) on Stack Overflow for
demonstrating a way to do an upsert without the UPSERT syntax. This
enables us to do an upsert on SQLite < 3.24.0. Neat trick.
https://stackoverflow.com/questions/15277373/sqlite-upsert-update-or-insert/38463024#38463024
|
|
Don't create a `merge_bases` entry in the database if no merge base
argument was passed to the script. The `get_merge_base` function in
`code-review-database` will default to `origin/master` or `master` when
no row is found in the database. Rely on those defaults instead of
always inserting a row.
|
|
Put these functions in a function-only file that can be sourced by other
scripts that need database access.
|
|
Function to get the merge base for the current branch. If none exists in
the database, default to `origin/master` if there's an `origin` remote,
or `master` if not.
Thanks to Serpiton
(https://codereview.stackexchange.com/users/61525/serpiton) on Stack
Overflow for describing how to get a default value using the `MAX()`
function:
https://codereview.stackexchange.com/questions/74192/select-first-value-if-exists-otherwise-select-default-value/74252#74252
Thanks to 'tig' (https://stackoverflow.com/users/96823/tig) on Stack
Overflow for the command to check if a given Git remote exists:
https://stackoverflow.com/questions/12170459/check-if-git-remote-exists-before-first-push/26843249#26843249
|
|
Turns out some files were excluded using the `..` range.
|
|
|
|
This command will store the desired merge base for the current branch,
defaulting to "origin/master". In so doing, subsequent `code-review`
subcommands will have access to this and be able to default to a merge
base branch instead of:
* Having users specify one manually at the command line for each
`code-review` subcommand
* Having a default merge base branch that's not desired by users
I figure I can hopefully get away with not sanitizing SQL input because
I don't think you can use apostrophes in Git branch names. Otherwise
I'll probably have to port this to another language with a SQLite
library, as it doesn't seem possible to easily sanitize inputs in Bash.
|
|
Fugitive seems to have fewer surprises and basically just work in Vim.
|
|
Output a short log of commits.
|
|
Outputs a stat of the changed files.
|
|
Since subcommands are separate programs, we need to make sure we pass
all remaining arguments along to the subcommand.
Without this, I wasn't able to pass a different base branch to my `diff`
subcommand.
|
|
Script to show a Git diff of the current changes. Uses `master..` as the
default base.
|
|
This script takes a subcommand as the first argument and executes an
executable called "code-review-${subcommand}", similar to the way Git
does subcommands.
Using this subcommand system, I can have different unrelated subcommands
in separate files/scripts, making it easier to organise the code.
|
