This is an update on the development of Flirt. Make sure to read the announcement post before this one, otherwise it won't make sense: Announcing Development on Flirt. According to the roadmap, my goal for December and January was:
Develop a reasonably detailed specification of the feature set, taking care to support a broad variety of backends while keeping the user experience across backends consistent. Implement this feature set for the "Git native" backend.
I have mostly achieved this goal, although the native backend is unfortunately not yet feature-complete. The basic mechanism of storing, sending and receiving review information via a Git remote is working though. Let's get into it!
Table of contents:
I analyzed three code review platforms for their features: GitHub, the mailing list and Gerrit. Many others (Gitlab, Forgejo etc.) are similar to GitHub, so I should be able to support them later without too much trouble. I won't reproduce my entire analysis here, but only touch on some interesting features that will or won't be supported. Let me know if you disagree with anything or you think I forgot to consider something!
Flirt wants to encourage code review on a per-commit basis. Commenting on the combined diff of a multi-patch Spirit is therefore discouraged. In fact, I'm planning to not support it at all.
That raises a question though: How is Flirt going to deal with existing comments on the full diff, e.g. ones made in the GitHub PR UI? I think there are some decent fallbacks, but I haven't decided on a specific one yet. I could treat them the same way as "free-standing" comments, which is something Flirt will need to support anyway. (For example, a regular comment that's not attached to a diff at all in the GitHub "Conversation" tab.) Another option is to treat it as a comment on the last patch of the Spirit. The last patch may not have changed the line the comment was made on, but at least the state of the code would match. And supporting comments anywhere in the codebase is desirable anyway. For example, if a Spirit author forgot to update documentation, Flirt should allow reviewers to comment at the correct location in the documentation, even though the patch doesn't touch the documentation at all.
Gerrit has an interesting feature, which allows you to comment on a range of characters within a line. (Actually, it can even be a range spanning multiple lines, but starting and ending at arbitrary points within a line.) I think this can be useful in some situations. For example, I sometimes see markdown files in the wild where a whole paragraph is written on a single line. In that case, commenting on that line may not be specific enough, since the comment could apply to any part of a whole paragraph.
I'm not planning to support this though, because I'm not convinced this is a widely required feature. (Gerrit users: Let me know if I'm wrong!) In the case of my markdown example, it seems straight forward to tell the author to follow the "one sentence per line" rule or hard-wrap the text at a specific line-width.
Flirt will support commenting on ranges of lines though, that's commonly supported by backends and generally useful.
... continue reading