Melpa: MELPA review bot

Created on 20 Feb 2020  路  7Comments  路  Source: melpa/melpa

Hi!

I continued to comment on the MELPA PR for some days since this comment.
And I realized that this job was harder than I expected.

Unfortunately, some people forget the basics before sending their PR that their packages can be byte-compiled without warnings, have the proper docstrings attached, run package-lint, etc...

Also, even though it was running, I could see a warning, but the package owner sometimes thought it was clean (maybe some customization hide warnings?).

I think the need to introduce a simple review bot to solve such problems.
I have felt this necessity as a package owner, but I realized that by doing some job of the reviewer.

I think this bot will be a good bot for package owners.
Unfortunately recently, after submitting a PR, it was often left without any comment for nearly two weeks. After a two-week wait, it's hard to point out some issues that static checkers can detect.

To implement this feature, we need to run the bot in the event of opening the PR and commenting on the PR (re-review comment or something trigger action).
It seems to be possible using some technique, but it is easy to use a webhook that can be set by the only repository maintainers.


feature

Most helpful comment

Hi, I just want to thank @purcell and @riscy for their efforts to melpa and Emacs community. And I just started a project for automated review new package PR on melpa. Here is the link to the repository -> jcs-elpa/package-bot.

This is the flow chart on how currently this program works.

flow_chart

And here are results -> jcs-elpa/package-bot/pulls/1 from recent PR (I have all the package reviews on this PR just for testing purpose).

Some guide to this program

  1. If you care about the review process, please checkout the file review.el at the root of the repo.
  2. If you care about the template, please checkout files under templates folder.
  3. For other configuration, please checkout the file config.js.

P.S. This program uses NodeJS.


Let me know what you guys think! Thanks!

All 7 comments

I feel like this issue is redundant, because #4154 and related discussion covers most of what is needed. It's not a matter of a bot, IMO, it's integration of package-lint into the existing CI build process. For this, we need:

  • package-lint to handle multi-file packages (see https://github.com/purcell/package-lint/issues/111)
  • Reliable detection of which packages have been affected by a particular pull request. There's some logic for this already, but occasionally people's PRs include many old commits which affect other recipes, if the PR branch was based on an old MELPA commit.
  • A hook for the package-build process which builds modified recipes, so that package-lint is run on the packaged files (not on the source files).

So that's already a bit of work.

Additionally, we have tried to avoid byte compilation of packages, since this is a security risk, but the review process should ideally include byte compilation. How to integrate this safely into the build is an open question.

What @riscy has been doing with his semi-automated checks has been quite helpful, so it would be good to maybe make that process more self-service for elisp authors, or for other volunteer reviewers.

Overall there's always a final human judgement of whether/when to merge, so there will always be a bottleneck. The way to eliminate that is to have an actual handbook which covers everything to look for, and our various de-facto policies, so that more reviewers can consistently apply the MELPA conventions to submissions.

Thanks for organizing the points, that list is something we don't yet have.
Indeed, it seems like a good idea to build into a build system rather than develop a bot.

Yes, there is huge danger for Elisp byte compilation.
At the beginning of the review, I was reviewing a package submitted in Emacs, which was isolated from the network, but recently I open as it is...

Such dangerous tasks should be performed in an environment isolated from userspace, such as a CI environment.
Therefore, if there is a dangerous eval-{when/and}-compile, I also want the ability to label it.

Hi, I just want to thank @purcell and @riscy for their efforts to melpa and Emacs community. And I just started a project for automated review new package PR on melpa. Here is the link to the repository -> jcs-elpa/package-bot.

This is the flow chart on how currently this program works.

flow_chart

And here are results -> jcs-elpa/package-bot/pulls/1 from recent PR (I have all the package reviews on this PR just for testing purpose).

Some guide to this program

  1. If you care about the review process, please checkout the file review.el at the root of the repo.
  2. If you care about the template, please checkout files under templates folder.
  3. For other configuration, please checkout the file config.js.

P.S. This program uses NodeJS.


Let me know what you guys think! Thanks!

Anyway, I may not want to run this program with my account since I will get all notifications from each PR (I personally don't want to). I think this would be great if anyone who is in the melpa organization can take this? Maybe like melpa-bot or something similar. Or to anyone who willing to do this! Thanks!

Could we create a Github action that runs this bot, or something similar, and ask package owners to make that part of the build on Github?

It won't fix _all the packages_ since some aren't on Github, but that would be a start..

This is experimental and might break without warning, but I've written one action for the checks I use.

This could be useful for help with this issue

Was this page helpful?
0 / 5 - 0 ratings