Re: Add zuul.el package to ELPA

[Niklas Eklund]

Philip Kaludercic <philipk@posteo.net> writes:

> Niklas Eklund <niklas.eklund@posteo.net> writes:
>
>>
>> Hello,
>>
>> I would like to add a package that I have developed named zuul.el,
>> https://git.sr.ht/~niklaseklund/zuul.el, to ELPA.
>
> Just a few comments from briefly skimming the code:

Hi Philip,

thanks a lot for the review! :)

> I believe if you used compat, you can reduce the minimal version of
> Emacs down to 26.1, and the only hard dependency appears to be
> project.el?

I wasn't familiar with the compat library. But if I have a dependency to
project.el which package-lint tells me I need to have Emacs 28.1 for, is
the proper way to add project to Package-Requires to circumvent the
need? Should I then do the same for e.g. seq or does compat provide
functionality for that library?

> I'd also recommend using more specific custom types for your user
> options.  See (elisp) Composite Types.

Thanks for the suggestion, I wasn't aware about these composite types!

> It also appears you should be able to use defconst and defvar-local in
> your private variable section.

defvar-local I didn't know about.

> Keywords (:foo) do not have to be quoted to prevent evaluation.
>
> I believe, but I might be mistaken, that it is advised to require rx in
> a `eval-when-compile' block.

Make sense.

> (mapconcat #'identity ...) should do the same as `string-join'

Yeah this becomes clearer.

> Defining a map like zuul-log-mode-map is usually done using defvar
>
>     (defvar foo-map
>       (let ((map (make-sparse-keymap)))
>         (define-key map ...)
>         ...
>         map))

I have taken your suggestions and made them into the following commits:
- Add defvar to zuul-log-mode-map
- Replace mapconcat with string-join
- Require rx at compilation
- Use defconst and defvar-local in private vars
- Use composite types for defcustom variables
https://git.sr.ht/~niklaseklund/zuul.el/log

/Niklas

>> The package allows users to access build logs from the Zuul,
>> https://zuul-ci.org/, CI system. Users uses the completion interface of
>> Emacs to select a specific build and a buffer is then opened with the
>> build log.
>>
>> The package utilizes Emacs compilation mode to navigate between errors
>> in the logs. It also lets the user configure where projects can be found
>> on the local machine so that navigating to the source code of an error
>> works, even though the absolute path might not be found on the local
>> host.
>>
>> /Niklas Eklund
>>
>> From ed4201c73aa66b21b040169009dddb636ab76048 Mon Sep 17 00:00:00 2001
>> From: Niklas Eklund <niklas.eklund@posteo.net>
>> Date: Mon, 29 Aug 2022 20:26:35 +0200
>> Subject: [PATCH] * elpa-packages (zuul): New package
>>
>> ---
>>  elpa-packages | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/elpa-packages b/elpa-packages
>> index 4ae12a26ca..5ab182f93b 100644
>> --- a/elpa-packages
>> +++ b/elpa-packages
>> @@ -810,4 +810,8 @@
>>   ("yasnippet-classic-snippets" :url nil)
>>   ("zones"           :url nil)  ;https://www.emacswiki.org/emacs/zones.el
>>   ("ztree"           :url "https://github.com/fourier/ztree";)
>> + ("zuul"            :url "https://git.sr.ht/~niklaseklund/zuul.el";
>> +  :news "CHANGELOG.org"
>> +  :readme "README.md"
>> +  :auto-sync t)
>>   )