[ANN] Egerrit - An Emacs Gerrit package

Hi everyone, I want to share I package that I have developed, which is **egerrit**, an integration of the Gerrit Code Review inside of Emacs. For more information and screenshots see my blog post [https://niklaseklund.gitlab.io/blog/posts/gerrit\_in\_emacs/](https://niklaseklund.gitlab.io/blog/posts/gerrit_in_emacs/) Here is some of its features: * A customizable dashboard * Gerrit specific actions, such as setting WIP, topic, rebase a change etc. * A review mode, with the ability to review a patch diff and add comments that can be published * Functionality to automatically checkout out a copy of the change that is being reviewed so the change can be explored in its completeness * A list of conversations with the ability to use it to jump directly to source code when interacting with feedback on a patch of yours The package only uses built Emacs functionality, so its easy to install. If it sounds interesting feel free to try it out, the source code can be found here [https://gitlab.com/niklaseklund/egerrit](https://gitlab.com/niklaseklund/egerrit) :)

Thank you! I haven't tested it yet. Personally, I find gerrit interface very very sluggish, so maybe this would be a great improvement.

I have never heard of gerrit and therefore never used it, but I have to say this is impressive. It seems like this package is well thought through and has a huge feature set. Impressive work!

Thanks! If you ever cross path with the tool in the feature you know where to look :)

Thanks! Would be interested to hear your feedback if you give it a go :). I have so far only have other Emacs users at the company use the package, so quite a limited user base :P I get that feeling from time to time as well. For some larger patches I have noticed that it has been much more rapid to view them inside of Emacs.

I've been looking for something like this! I'll give this a test drive at work. Thanks!

Would be great! :) If you run into any issues or have questions feel free to write them here or open an issue on gitlab.

Hi! gerrit.el author here, Thx for writing a package that supports performing gerrit reviews from within emacs! This is IMO the first package that supports that. I'm curious what you didn't like about gerrit.el in case you had a look at it (at least you have mentioned gerrit.el in the README, thx for that!). Is it that it depends on non-stdlib libraries/packages like e.g. magit, which does not allow adding the package to GNU elpa, or is it sth else? I'm still actively improving gerrit.el and would love to get some feedback from you. Since our packages share a lot of features (the dashboard e.g.) I think it makes sense that you e.g. highlight the differences between the two packages and maybe even think about merging them. I have one further question, why does your pkg depend on curl instead of on the builtin url.el package?

Hey, thanks a lot! :) Also nice of you to reach out! Its a good question. I don't think your package was around when I initially looked for an alternative, or it was just me how didn't find it. So I have been working on this in isolation and it was just recently that I realized that gerrit.el existed. At that point I didn't see much difference, and I have also built a specific extension for my current work on the package I have developed so I didn't have the incentive to switch :) No, it didn't have anything to do with that, I personally like that it could be brought to ELPA, but that had nothing to do with not choosing your package. To be honest I haven't tried your package yet, I should probably do that before I could come with some proper feedback :) It would be nice to collaborate, and perhaps look into the possibility to merge the two packages. As you say the packages share some similarities but we have also implemented them differently so would be great to share our insights about our developments. I also believe the community would benefit from having one package instead of two. Do you want to continue the discussion on github/gitlab or in a pm? About your question on using curl it was more of a coincidental choice then a deliberate one, using the builtin url.el sounds like a more reasonable approach! Cheers

Thx for your answer! I'll also test your package in the next weeks and will compare it with gerrit.el. I'll also create a ticket/issues in egerrit.el, which we can use to continue our discussion about the collaboration.

Great, sounds like a plan! :) If you want I am happy to have a video session walking through the way I use my package. But if you prefer to explore it yourself that's fine as well :)

Thanks for the package. I've tried to use egerrit recently. But I can go only as far as opening buffer `*egerrit-dashboard*`. When I hit enter on one item (which runs command `egerrit-author-review-change`), the following error occurs: egerrit--add-change-jobs: Symbol’s function definition is void: nil Any idea what might go wrong?

Hi there, thanks for trying out the package and reporting the issue you encountered! I am guessing what is wrong is that the regular expression that is used to extract the jobs from messages posted on the change is nil. [https://git.sr.ht/\~niklaseklund/egerrit/tree/main/egerrit-job.el#L63](https://git.sr.ht/~niklaseklund/egerrit/tree/main/egerrit-job.el#L63). What I would propose that you do for now is to set the **egerrit-job-function** to nil [https://git.sr.ht/\~niklaseklund/egerrit/tree/main/egerrit-job.el#L68](https://git.sr.ht/~niklaseklund/egerrit/tree/main/egerrit-job.el#L68), that will disable the inclusion of CI jobs but see if that can make the rest work for you? Make sure you get the latest commit from the main branch because I just pushed a fix to handle when egerrit-job-function is nil so that it doesn't crash on that. Also feel free to create a thread here [https://lists.sr.ht/\~niklaseklund/egerrit](https://lists.sr.ht/~niklaseklund/egerrit) if you encounter more issues then its easier for others to find it later I hope :)

`(setq egerrit-job-function nil)` works. Now I can try more features.

Nice! Looking forward to some feedback :)

This package is quite usable! Sadly I don't have a patch in review now, so I have to try some features later. Here are some issues I have: 1. Didn't find a way to make a file comment 2. Kill the `*egerrit-conversations*` buffer seems to delete all the unpublished comments, is it designed like this? So it's different from the draft on the web page (as the drafts are saved on the server)? 3. Don't know how to switch to a previous patch set rather the latest one. 4. I'm not sure when the `+1` operation will be published. The moment `egerrit-set-review-label` call finished or will be published together when calling `egerrit-publish-comments`?

Thanks for the quick feedback :) 1). Good idea, I haven't considered this, I guess I don't make that many file comments. Maybe selecting a file header in the diff buffer should do this? If selecting the +++ it will be on the revision --- will be on the base? 2). The current implementation does not save any unpublished comments. So as you say the workflow is to compose the comments and then publish them. Maybe you would like to have the option to publish draft comments only? 3). To switch to another revision you use the **C-u** prefix argument. So when you review a patch you would do **r C-u <return>**. And to select a different base to compare to you use the **partial review** 4). Hmm I will rename that command to publish review. The name set sounds more like you describe that it should be published with the comments, but it in fact sends the review directly to the Gerrit server. Great to get some new eyes on the workflows here, I have grown quite accustomed with my own implementation :P. I will push some updates today for some of the easier things here.

1 - I can think of 3 ways: * Comment anywhere on the filename line * Create a new command, and let the user select files changed in the patch. * Change the UI totally. Display/Review one file a time, instead of using the diff view. 2 - Web UI and egerrit both let the user create drafts first, then publish them altogether. But web ui allows user to close the browser and come back to it later. This can be useful if the patch is relatively big, and the review want to relax for a while, or simply interrupted by something else. 3 - This is a very important feature. Should be mentioned in the function descriptions. 4 - In web ui, I like to reply comments and `-1` together, to indicate that's the reason that I vote `-1`. Is it possible to add a section in conversation buffer to indicate the `-1`? I'm thinking of something like the `TODO`/`DONE` for a single comment.

5. To be honest, I didn't feel the difference between command **egerrit-author-review-change** and **egerrit-review-change**. And I think, after the patch is pushed to the gerrit, there isn't too much difference between the author and reviewer. For example, someone not the author can also push a patchset using the same Change-Id.

Actually the difference is quite subtle. Maybe it can be enhanced, I haven't given it to much thought. There is an important difference though, the \`egerrit-review-change\` will behind the scenes download the actual patch and check it out in a dedicated branch. I heavily rely on this when I review patches since it allows me to jump directly to the source code and see the change in its entirety, or navigate to the definition of some new function for example. Then when you finish the review and go back to the dashboard egerrit will checkout the branch you were on prior to the review. With the author command the difference is that it leaves it up to the user to make sure you have the correct patch checked out. This I use if I want to look at a patch that I am the author of, and that I have already checked out. Maybe I am in an interactive rebase and don't want egerrit to tamper with git. Do you think other names would clear up this, or you think there is potential to add more features to the different commands then what they currently feature?

1). I think either select the filename or comment on a hunk without highlighting the line or a region, then it becomes a file comment. 2). I have a very crude "database" implementation which just writes lisp data to file and later reads it back. Egerrit uses it to store review labels so that its easy to see later which patch sets that one has previously looked at when doing a partial review. If I would change the workflow so that the quit command always is necessary to quit a review, but that comments that has not yet been pushed could then be written to a database, then later when reviewing it would be easy to restore unpublished comments. Its a good idea. I will write that down. 3). Thanks for pointing that out, will update the documentation :) 4). That would be possible. I have been thinking of adding like an information section at the top of that buffer, there are things that could be valuable to show, one of them could be the review label. My workflow at the moment is that I create all my comments then do a **C-c C-r** to publish a review label and **C-c C-s** to send the comments. But I think a better workflow would be what you are mentioning having the ability to add all things locally and then a publish command that figures out what different requests that needs to be sent to Gerrit

I added a [notes.org](https://notes.org) file to the repository where I have documented ideas for improvements as well as new features. https://git.sr.ht/\~niklaseklund/egerrit/tree/main/notes.org#L1

It seems to me that the point is whether the reviewer already has the reviewing source code checked out. I always think making changes to git branches programmatically can be tricky. There are too many situations has to be thought of. I actually forget to call `egerrit-quit-review-or-feedback` once, and wonder where does the branch come from. Do you think it's better to ask the user to confirm the download and checkout? Or at least notify the user with a message.

I find 2 other features that I used a lot, but not know how to do it in egerrit: 6. Change the patchset diff base. Sometimes I made a comment on a certain patchset, say patchset 3. And the author fix it in patchset 5. I always compare beween patchset 3 and patchset 5 to see what has been changed. 7. I sometimes would refer/comment part of a file that's not changed. The `egerrit-diff-mode` is not convenient to do this, if possible at all.

Yes correct, that's the difference at the moment. Its a bit tricky indeed but it has worked better than expected for me personally. I think there are some edge cases I need to fix though. One is entering a review and quitting it before it has finished to checkout the new branch in the background. This currently causes an error message. For forgetting to quit a review I think this can be improved when there is support for pausing or resuming a review. Then egerrit can prompt what you want to do when you try to do a new review without quitting the last one? Maybe, but when I do my reviews I always want it to checkout the code. Maybe it can be a setting though, to ask or not. It will notify in the current implementation, it says checked out branch ... And switched back to branch ... When a review is quit. Another feature to use is the :review-repo that can be added to a project config. That allows for all review commands to use a copy of the project, which can be dedicated for reviews. Never any problem with git when you do that for me :)

1. This is what egerrit calls a review-partial-change accessible with **r S-<return>** or without the r for author version. Then it prompts you for a revision other than base and compares that with the latest revision. If a prefix argument is provided it will prompt for the second revision and not just assume latest. 2. Yes, this is not possible at the moment. My idea for this is that since its very easy to just press return on a hunk to see the actual source code in the other window. That's typically how I do it if I want to see the full context. Then the plan is to create a global minor mode that will allow one to start conversations based on a region of code in the source file. What do you think about that idea?

1. Thanks. 2. I like the idea. In my imaginary gerrit reviewing in Emacs, I always picture something similar to `ediff`.

So that what's what `:review-repo` for! I see now. :D Maybe you already thought about it, but let me still mention that magit use a strategy, that make a buffer with name like `lisp/edmacro.el.~5b1109394c^~`.

Yes :D Aha, when you are visiting the source code you mean? I actually prefer the way it is now, checking out the full change. Because then I know everything will work when I visit the source code. The language servers will pick up the changes so I can goto definitions. I might also want to propose a different way of writing some piece of code so now I can make the change in the source code, with completion, and copy that to the conversation I am starting :)

I think in this way, checking out the patchset can be optional. In my work, we have many projects. * Some of them I don't even have the environment to build it, but may still need to review it (the result can be checked through CI output). * Sometimes it's only a document patch. * Sometimes I'm just helping other teams to review certain part of a patch. In many scenarios, I don't want to check out the patch.

I was imagining something like that myself as well, being used to the split screen configuration of gerrit. Coincidentally I started with building on diff, because that I could grasp how to implement :P. I do think this way is more powerful now when I have been using it for a while. Having a compact patch diff complemented by a window with the source code that will be the result of that patch applied. And then having a buffer dedicated to conversations. Also having imenu in the diff buffer is pretty great IMO, much quicker to switch to a specific file with completion. I have the idea that it should be easy to implement functionality to go to the alternate file, for example visiting a header file and then jump to the .cpp file, when in the diff buffer. Although i think the split review thing the the web ui on occasions can have the upper hand in clarity of how a patch effects the code. It happens rarely IMO but it do happen. But thinking about it, it might be possible to implement functionality to given that you are in a diff buffer switch to an alternative view of the file you are reviewing. It should be possible to base the alternative view on ediff.

That makes sense, I personally work in a monorepo structure so its good you provide with feedback from other setups. Then I think its good that it is optional. Maybe this is a matter of just renaming the author/review commands so that their names reflect better what they do?

I think renaming should be good, at least for now. (Maybe rename to something like `egerrit-download-and-review-change`? Not very sure.) BTW, I'm reading the source code now (but very slowly). Hope I can contribute someday. :D

I will look into it :) Cool, contributions are much appreciated :D Btw, I thought I should mention that for changes that you are the author of I recommend using **c** to only open the conversation buffer, then navigating between the conversations brings you to the source code where you in some cases would want to make changes based on the feedback from a reviewer.