Skip to content

cwebber/magit-review

Folders and files

NameName
Last commit message
Last commit date

Latest commit

 

History

41 Commits
 
 
 
 
 
 

Repository files navigation

#

About magit-review

Motivation

For a long time I used magit-wazzap to handle all my code reviews. magit-wazzup is awesome… who doesn’t want a buffer where they can see all branches with new commits for them to check out at once?

But I found that as my project grew, magit-wazzup failed to scale for me:

  • It was too slow. My project had about 500 or so branches and magit-wazzup would check all of them if they had new commits and format them all for display. magit-wazzup had an interface to mark something as “ignored” but it didn’t really work for me because the branches that should have been ignored didn’t show up for me to ignore them anyway!
  • I also found magit-wazzup’s ignore tool annoying because I didn’t have a nice way to double check later that I really wanted things that were ignored to stay ignored.
  • Sometimes I wanted to mark a few branches as “I need to review these” and jump to a limited view of wazzup so I could just focus on the branches I knew needed attention.

If these irritations sound familiar to you, you might like magit-review (if not, you might want to just stick with magit-wazzup; it’s admittedly slightly simpler). And if you already like magit-wazzup, you’ll be happy to see that magit-review basically works the same way, with just a few small enhancements.

magit-review’s strategy

magit-review works pretty much the same as magit-wazzup except that it adds two features: states and filters. You can mark a branch with some sort of state (magit-review will serialize this so that it’s remembered) and apply a filter so that only branches that match that state actually show up. For more information on how to actually make use of that strategy, read on.

License

magit-review is licensed under the GNU GPL v3 or later, just like GNU Emacs.

This document is waived into the public domain under CC0 1.0.

Using magit-review

Installing

Put magit-review.el on your elisp path or something. ;)

Honestly, I just do:

(load-file "~/devel/magit-review/magit-review.el")

Invoking magit-review

First, open magit:

M-x magit-status

Next, open magit-review:

M-x magit-review

At this point you should have an interface that looks exactly like magit-wazzup, except you’ll notice there’s a header above all the commits that says “Branches in unknown:”

This is because you haven’t filed any branches into any states yet. Let’s figure out how to switch filters and states so you can make the most of magit-review.

States and filters

Switching filters via bookmarks

Filters, as said, switch our branches based on their state. There’s a small rule syntax for switching the active filters, but we’ll avoid going into how that works for now. For the moment, let’s try switching filters the easy way.

The default filter is “general” mode. We’ll explain the details of how filter rules work later, but the rule for this is:

"tracked=all ignored=none other=new"

This means that:

  • Every branch that is marked to be “tracked” is shown, whether it has new commits or not. (If you haven’t changed any states, nothing should be tracked yet because you haven’t marked anything as such.)
  • Anything marked as “ignored” will be ignored whether it has new commits or not.
  • Anything else will be checked first wither it has new commits. If it has new commits, it’ll be shown; otherwise it won’t show up at all.

Let’s try switching to another filter. Press the “t” key. This brings up the fil”t”er bookmarks menu. Try pressing “a” for “all”. If you have branches that have no new commits in them, these will also now show up. You can switch things back by pressing “t” and then “g” to switch back to the “general” filter again.

Changing a branch’s state

But maybe that general view takes too long to load. Augh! There’s plenty of branches that just don’t have anything anymore. Why should you wait for them to show up when you know they don’t have any commits?

So, let’s switch to the “nothing new” filter. This will help you hunt for branches that don’t have anything new in them anymore so just shouldn’t be displayed. Press “t” then “nn” to switch to the nothing new filter now.

Are there a bunch of branches here that say there are no commits? On mine there are, and they look like this:

(no commits) master (willkg)

(no commits) keyboard_nav (willkg)

(no commits) 401-plugins (willkg)

(no commits) sqltests (tryggvib)

Okay. So we don’t need these to show up any more, so let’s tell magit-review not to look for them anymore. We want to switch them to the ignored:nothing-new state. That way they won’t take up any more time when we’re in the general filter.

Move your cursor over one of these branches and press “s” to bring up the states bookmark menu. Press “in”… this should set the state to “ignored:nothing-new” which means “we’re ignoring this because we don’t think this has any new commits in it any more.” (You could set up a filter to double check that you’re right and that these branches haven’t somehow gotten new commits later… magit-review makes that easy.) The color of this branch should change indicating its state has changed.

We can verify that this branch is moved over to the “ignored:nothing-new” state by switching our filter. Press “t” then “ia” to switch to the filter that shows all branches currently marked as ignored. You should see the branch you just marked show up.

Okay, awesome. Maybe you’ve changed a bunch of branches now. Switch back to the general view (“t” then “g”)… if your situation is like mine was, it should load a lot faster now!

But wait… augh! There’s still some branches here that we just don’t care about any more. On my general view I currently see this branch:

4 unmerged commits in flatpages (willkg)

Ugh! We merged that branch! The problem is that we rebased it locally before merging, so it looks like it has new commits. But it doesn’t really! We don’t want it to bother us anymore when we’re looking at our general review overview… let’s get it out of here!

Type “s” then “ii”… this switches the state to “ignored:ignored”, which means that it doesn’t matter to us that this has new commits, we just don’t ever want to see it ever again because it’s irrelevant now.

Awesome! If you hit “g” to refresh the buffer, it shouldn’t show up any more.

Okay… wait a second, did you notice that the two states that we set both started with “ignored:”? That’s because states generally come in two pieces: “general:specific”. For example, the general workflow of magit-review is that we either want to track things or we want to ignore them. But we might want to do so for different reasons. For example, we wanted to ignore some branches because they didn’t have any new commits, and we wanted to ignore some other branches because they do have some new commits but they’re just not relevant anymore. It’s important to have those distinctions (okay, it’s important to me): if new commits appear in the ignored:nothing-new section we’ll want to find them and move them out of there. That’s not true of ignored:ignored because we want to ignore them forever.

We’ll explain the meaning of the builtin states and filters in the next section, but for now let’s try doing one more state thing: sometimes you want to mark something as being tracked so you can focus on a shorter list of things to review. Let’s try that: select a branch you need to review. For example, in my case I have a branch that my friend spaetz has been requesting I look at, and I’m like, yeah yeah, it’s on my radar! I’ve hit <Tab> on this one to look at the list of branches, so it’s expanded. Locally, it looks like this to me:

3 unmerged commits in WIP/large_uploads (spaetz)
aab5af4 * Don't read full image media into RAM on copying (#419)
f2abb7b * Make Cloudfiles copy memory efficient too (#419)
f9b5d9c * Make copying to/from storage systems memory efficient (#419)

But let’s really move it onto my radar by marking it as tracked. Hit “s” then “tr” which moves it to “tracked:review”. Now I can filter to seeing just tracked:review things… or even in the general section, this branch shows up in a completely different section than the ones that aren’t marked:

Branches in tracked:review:

3 unmerged commits in WIP/large_uploads (spaetz)
aab5af4 * Don't read full image media into RAM on copying (#419)
f2abb7b * Make Cloudfiles copy memory efficient too (#419)
f9b5d9c * Make copying to/from storage systems memory efficient (#419)

Whew! I’d better get to that one soon. ;)

Builtin states and filters and their meanings

The default list of states is fairly short. As said, states fall into two categories: things to review and things to ignore (you don’t have to use these paradigms, these are just the default ones).

  • tracked:review – This is something that needs to be reviewed and merged. It’s on a “shortlist” of things you need to look at. You’ll get to it… you promise!
  • tracked:deferred – This is something you’re keeping an eye on but which isn’t on your immediate review queue. Very likely, you’re waiting on something. For example, you may have reviewed it and passed it back to the original author and are asking them to make some changes before you are ready to merge it.
  • ignored:nothing-new – The last time you looked at this there weren’t any new commits in it. That doesn’t mean there aren’t now… somehow that might change (you may want to use a filter so you can check on this from time to time and pull things out of this state) but at the very least don’t slow down our general view by scanning them for new commits.
  • ignored:ignored – There may be new commits in here or there may not… you really don’t care, you just don’t want to see these branches anymore.

There’s also a “special state” which is “unknown”… which means there is no branch state set at all. (You can filter on this special state, and switching the branch to unknown will actually clear the state altogether.)

Switching states and filters manually

If you want to set a state manually, that’s really easy. Instead of hitting “s” to use the state shortcuts, use “S” instead to change it manually.

Similarly, instead of using “t” to change the filter via a bookmark, you can hit “T” to change it manually. However, before you do that, you should probably understand how filters work!

Understanding the filter syntax

The filter syntax is very minimal. It basically works like:

"state=directive another:state=directive"

In other words, you set multiple filter components separated by spaces. Each filter component has a state (yes, you know what states are by now) and a directive that says what should be done with whatever branch matches that state. Each filter is looked at in order; the first rule that matches is accepted.

So let’s look at a real-world example.

"tracked:review=all ignored=none unknown=new other=none"

Okay, this is simple! (It could be written more concisely, but I wanted a comprehensive example.) Let’s look at each piece in turn.

  • tracked:review=all – If something is marked as tracked:review, this matches. The directive is “all” so it will be shown regardless of whether it has new commits (useful… you can move anything that no longer has new commits because it was merged into ignored:nothing-new if you forgot about it)
  • ignored=noneAny branch that is in the ignored general state will be ignored. It doesn’t matter if it’s in ignored:ignored or ignored:nothing-new, it will be ignored. Notice how this is different from the above rule, which is much more specific about state, while this is more general.
  • unknown=new – As said above, unknown is a special state meaning that it has no state. According to this rule, only branches with new commits will be shown if they have no assigned state.

There’s also one rule that doesn’t need to be in this filter, but it’s here for demonstration purposes:

  • other=none – This rule is somewhat superfluous here because “none” is the default directive for anything that doesn’t have a matching rule (you can change this by changing the variable ‘magit-review/default-directive if you like). “other” is the other special state, and it’s a catch-all (so it should always go last… magit-review isn’t smart enough to reorder things so anything after this rule is effectivel ignored). So as you can probably guess, in this case, something in say tracked:deferred would be caught by this rule and be told not to be displayed (even though that’s the default anyway…)

Valid directives are:

  • all (show everything)
  • none (show nothing)
  • new (show only things with new commits)
  • nothing-new (show only things that have no new commits)

Knowing this, we can create some fancy filters. For example, to skip over everything ignored but otherwise show anything that has new commits:

"ignored=none other=new"

Or, to double check that our pile of branches in ignored:nothing-new really doesn’t have anything new in it (who knows, that could have changed when we weren’t looking!):

"ignored:nothing-new:new"

Likewise, to clean out stuff you tracked but that actually doesn’t have anything new anymore (so you should move it to ignored:nothing-new):

"tracked=nothing-new"

Note the tricky distinction between ignored:nothing-new the state and nothing-new the directive. ;)

Making your own state and filter bookmarks

As said, you’re not restricted to the states and filters that come packaged with magit-review… they’re just the states and filters that make sense to the author. But say you want to use the following states:

  • workified:do-it-now – This is when your boss is yelling at you because you needed to get this done yesterday!
  • workified:review-it – This is something at your work that you need to code review. You should probably look at it, you promised Sal that you’d check over her code soon!
  • workified:waiting-review – This is something that you passed off for someone else to review, or something. I don’t know, you come up with the rules. ;)

That’s fine! Go for it. Go ahead and do it! You can just set these states manually by using “T” and everything will be fine and great! ;)

However… you might want to not do this manually. That’s fairly tedious! So we have an easy solution: use bookmarks!

Bookmarks have a fairly simple syntax. Here they are as defined in the magit-review source code:

(defvar magit-review/filter-bookmarks
  '(("g" "General" "tracked=all ignored=none other=new")
    ("tr" "Tracked review" "tracked:review=new other=none")
    ("ia" "Ignored all" "ignored=all other=none")
    ("ii" "ignored:ignored all" "ignored:ignored=all other=none")
    ("in" "ignored new" "ignored=new other=none")
    ("nn" "nothing new" "ignored:nothing-new=none other=nothing-new")
    ("a" "All" "other=all"))
  "Modify this to change the keyboard keys which set the current filter.

Works like:
  ((\"shortcut\" \"Description\" \"state\"))

Note that after running this you probably want to eval
  (magit-review/add-filter-bookmark-keys)")

(defvar magit-review/state-bookmarks
  '(("tr" "tracked:review" "tracked:review")
    ("td" "tracked:deferred" "tracked:deferred")
    ("ii" "ignored:ignored" "ignored:ignored")
    ("in" "ignored:nothing-new" "ignored:nothing-new")
    ("c" "clear state" nil))
  "Modify this to change the keyboard keys which set which state.

Works like:
  ((\"shortcut\" \"Description\" \"state\"))

Note that after running this you probably want to eval
  (magit-review/add-state-bookmark-keys)")

Fairly simple in both cases. So say you wanted to add some state bookmarks so you can set these really easily.

(setq magit-review/state-bookmarks
      (append
       magit-review/state-bookmarks
       '(("wd" "Work: do it now" "workified:do-it-now")
         ("wr" "Work: review it" "workified:review-it")
         ("ww" "Work: waiting on review" "workified:waiting-review"))))
; You must run this in order for the bookmark keys to get regenerated
(magit-review/add-state-bookmark-keys)

(setq magit-review/filter-bookmarks
      (append
       magit-review/filter-bookmarks
       '(("wd" "Work: do/review it queue" "workified:do-it-now=all workified:review-it:new")
         ("wa" "Work: all" "workified=all"))))
; You must run this in order for the bookmark keys to get regenerated
(magit-review/add-filter-bookmark-keys)

As you can probably tell in both types of bookmarks, the syntax is roughly:

'(("key" "description" "state/filter"))

Now you have your own workflow!

Where things get stored

Easy! magit-review stores things in a big ol json dump in .git/info/magit-review.

Things to do

Sorting the state categories in the display

Changing filters manually

Make sure all our docstrings are still accurate

Forbid users from setting the state “other”; that’s a special case

Notes

About

Tool for code reviewing in emacs' magit. Like magit-wazzup, but more powerful.

Resources

License

Stars

Watchers

Forks

Releases

No releases published

Packages

No packages published