| Message ID | d4bd4555b09366c28463643e16b272ad7ca4eed8.1659826927.git.matf@disr.it |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#57021] gnu: packages: Add nmail. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
Hello, > * gnu/packages/nmail.scm: Reorder module imports. Remove > duplicate imports. > (nmail): New variable. Fix style. Thanks for this first contribution. As you are adding a new package, the preferred way to proceed is to amend your first patch with those changes and submit a new revision (v2). The module re-ordering stuff should also be part of a new patch. In short, we would expect something like: * gnu: Reorder mail module imports. * gnu: nmail: New variable. Do you think you could send those two patches in a v3? Don't hesitate to ask for guidance here or on IRC if I'm unclear. Mathieu
Thanks, I'll try to do that. However I see that the nmail dev is currently fixing multiple bugs that were reported lately, so I think it would be best if I wait until the next release. Thanks for your help! M On 2022-08-12 10:53 Mathieu Othacehe <othacehe@gnu.org> wrote: > > Hello, > >> * gnu/packages/nmail.scm: Reorder module imports. Remove >> duplicate imports. >> (nmail): New variable. Fix style. > > Thanks for this first contribution. As you are adding a new package, > the > preferred way to proceed is to amend your first patch with those > changes > and submit a new revision (v2). > > The module re-ordering stuff should also be part of a new patch. In > short, we would expect something like: > > * gnu: Reorder mail module imports. > * gnu: nmail: New variable. > > Do you think you could send those two patches in a v3? Don't hesitate > to > ask for guidance here or on IRC if I'm unclear. > > Mathieu
Hi Mathieu Laparie, Thank you for your patch. Sorry it has taken so long to get to it. This patch looks good. It applies and runs and lints with few issues. I've re-rolled a version 9 patch on the latest master branch, updating the version and changing the formatting slightly. Regarding the second diff, I'm not sure what the justification is for changing cyrus-sasl in the same patch, so I've removed that part. If that change needs to happen then I would think it would make more sense as a separate patch. Kind regards, Christina
Awesome news, thanks a ton Christina! Have you tried sending emails without the `cyrus-sasl` patch? Unless the package changed since I submitted this patch, sending emails may fail because `cyrus-sasl` needs to be compiled with the `--enable-login` flag, which I think was not default. I haven't tested lately though because I was running `nmail` and `cyrus-sasl` from my private channel since a long time. Mathieu On 2024-04-21 16:03 Christina O'Donnell <cdo@mutix.org> wrote: > Hi Mathieu Laparie, > > Thank you for your patch. Sorry it has taken so long to get to it. > > This patch looks good. It applies and runs and lints with few issues. > > I've re-rolled a version 9 patch on the latest master branch, > updating the version and changing the formatting slightly. > > Regarding the second diff, I'm not sure what the justification is for > changing cyrus-sasl in the same patch, so I've removed that part. If > that change needs to happen then I would think it would make more > sense as a separate patch. > > Kind regards, Christina >
Mathieu <mlaparie@disr.it> writes: > Awesome news, thanks a ton Christina! > > Have you tried sending emails without the `cyrus-sasl` patch? Unless > the package changed since I submitted this patch, sending emails may > fail because `cyrus-sasl` needs to be compiled with the > `--enable-login` flag, which I think was not default. I haven't tested > lately though because I was running `nmail` and `cyrus-sasl` from my > private channel since a long time. Given cyrus-sasl has a lot of dependencies (as reported by guix refresh -l), it would be good to submit this patch separately, even though that change is required for some functionality in nmail.
diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm index 0bc6519dd2..24730d5eb3 100644 --- a/gnu/packages/mail.scm +++ b/gnu/packages/mail.scm @@ -456,19 +456,20 @@ (define-public nmail "07lkl5syx3l37dhsl41nhmjknhxqgmvwc4il4gygsnr333qk75c9")))) (build-system cmake-build-system) (arguments - (list #:phases #~(modify-phases %standard-phases + (list #:phases + #~(modify-phases %standard-phases (replace 'check - (lambda* - (#:key tests? #:allow-other-keys) + (lambda* (#:key tests? #:allow-other-keys) (when tests? (invoke "ctest" "--output-on-failure"))))))) - (inputs (list libetpan - xapian - sqlite - cyrus-sasl - ncurses - openssl - file - (list util-linux "lib"))) + (inputs + (list cyrus-sasl + file + libetpan + ncurses + openssl + sqlite + (list util-linux "lib") + xapian)) (native-inputs (list pkg-config)) (home-page "https://github.com/d99kris/nmail") (synopsis "Terminal-based email client")