| Message ID | 20210319192144.17799-1-contact@dhruvin.dev |
|---|---|
| Headers | show |
| Series | Add node-global-gradle-clean | expand |
On Sat, 2021-03-20 at 00:51 +0530, Dhruvin Gandhi via Guix-patches via wrote: > I've recently started using Guix System and it has been my daily driver for a > month now. Surprisingly, guix already has every package I need. I decided to > ask my friends about packages they'll need before they can start using Guix. > > I will try to submit patches of those packages in coming months. I am new to > guix, and am new to contributing code via patches. Let me know if you have any > corrections/suggestions. See ‘16.4 Packaging Guidelines’ and ‘16.6 Submitting Patches’ in the manual. > Dhruvin Gandhi (1): > Add node-global-gradle-clean > > gnu/packages/node-xyz.scm | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) There's an uniform style for commit messages. Example for when adding new packages: (start snip) commit 7382b1027a319e505be6cfcadf1f5bd761dadccc Author: Ricardo Wurmus <rekado@elephly.net> Date: Thu Feb 4 23:20:05 2021 +0100 gnu: Add r-shinyjqui. * gnu/packages/cran.scm (r-shinyjqui): New variable. commit 5ae09d7979a0696d862b9555314eab199f7ce576 Author: Ricardo Wurmus <rekado@elephly.net> Date: Thu Feb 4 22:41:35 2021 +0100 gnu: Add r-spelling. * gnu/packages/cran.scm (r-spelling): New variable. (end snip) (More examples in the git history) When defining a new package, usually a copyright line should be added at the top of the file. I prefer referring to the commit directly instead of by tag, as the commit is required for SWH fallback if the repo disappears. Is there any particular reason tests are disabled? Maybe add a comment "; No test suite." if that's the case. I'm not a fan of starting package descriptions with "This package is ...", even though plenty of plenty of packages in gnu/package/node-xyz.scm have such a description. A description from gnu/packages/guile-xyz.scm I like: "Guile-DSV is a GNU Guile module for working with the delimiter-separated values (DSV) data format. Guile-DSV supports the Unix-style DSV format and RFC 4180 format." Greetings, Maxime.
Le 20 mars 2021 04:51:41 GMT-04:00, Maxime Devos <maximedevos@telenet.be> a écrit : >On Sat, 2021-03-20 at 00:51 +0530, Dhruvin Gandhi via Guix-patches via >wrote: >> I've recently started using Guix System and it has been my daily >driver for a >> month now. Surprisingly, guix already has every package I need. I >decided to >> ask my friends about packages they'll need before they can start >using Guix. >> >> I will try to submit patches of those packages in coming months. I am >new to >> guix, and am new to contributing code via patches. Let me know if you >have any >> corrections/suggestions. > >I prefer referring to the commit directly instead of by tag, as >the commit is required for SWH fallback if the repo disappears. Sorry to contradict, but using a tag is much more readable and works well with Software Heritage. At least, the linter seems to work in that case. Have I missed something? Dhruvin, could you submit a new patch with Maxime's suggestions? If you have troubles with Rome items don't worry, since It's your first patch, we can change a few things for you before pushing :) Also, could you run (from the guix checkout): ./pre-inst-env guix lint node-global-gradle-clean And fix any error it reports? Thanks a lot!
> See ‘16.4 Packaging Guidelines’ and ‘16.6 Submitting Patches’ in the manual. Yes, I will go through the guidelines first before submitting next version of the patch with suggested changes. > There's an uniform style for commit messages. Example for > when adding new packages: > ... I saw commit messages. I initially thought the committers added them, as I did not know how the patches are applied. But now that you have mentioned this, it makes sense, I will use that format for commit messages. > When defining a new package, usually a copyright line should > be added at the top of the file. Will do that. > I prefer referring to the commit directly instead of by tag, as > the commit is required for SWH fallback if the repo disappears. Okay. > Is there any particular reason tests are disabled? Maybe add > a comment "; No test suite." if that's the case. Yes, there are no tests (as of 1.0.1). I will mention that. > I'm not a fan of starting package descriptions with "This package is ...", > even though plenty of plenty of packages in gnu/package/node-xyz.scm have > such a description. A description from gnu/packages/guile-xyz.scm I like: > > "Guile-DSV is a GNU Guile module for working with the > delimiter-separated values (DSV) data format. Guile-DSV supports the > Unix-style DSV format and RFC 4180 format." I borrowed synopsis and description from the global-gradle-clean package author, and changed them a bit to match existing packages in the node-xyz module. I agree that they can be more descriptive, and concise. I'll update them as well. I have a question. The package may be used by only a few users. I hope that is okay with guix. Is there a rule defined somewhere, stating what gets in this guix channel and what should not? PS: When I submitted this patch, it created another bug 47270. I read about what a patch series is and read about how to submit them to guix afterwards. 47270 may be closed in favor of 47269. I will follow the way specified in guidelines. Thanks Maxime for your suggestions. Regards, Dhruvin Gandhi
> Dhruvin, could you submit a new patch with Maxime's suggestions? Yes, I will do that. > If you have troubles with Rome items don't worry, since It's your first patch, we can change a few things for you before pushing :) I don't know what Rome items are. > Also, could you run (from the guix checkout): > > ./pre-inst-env guix lint node-global-gradle-clean > > And fix any error it reports? > > Thanks a lot! I built the package for 3 rounds to check if there's something wrong. I ran guix lint node-global-gradle-clean before submitting the patch. It showed no warnings/corrections. I will do the same before submitting subsequent versions of this patch. Thanks for mentioning this :) Regards, Dhruvin Gandhi
Le 21 mars 2021 04:48:47 GMT-04:00, Dhruvin Gandhi <contact@dhruvin.dev> a écrit : >> Dhruvin, could you submit a new patch with Maxime's suggestions? > >Yes, I will do that. > >> If you have troubles with Rome items don't worry, since It's your >first patch, we can change a few things for you before pushing :) > >I don't know what Rome items are. They are "some items" after my French autocorrect kicks in… sorry for the confusion! > >> Also, could you run (from the guix checkout): >> >> ./pre-inst-env guix lint node-global-gradle-clean >> >> And fix any error it reports? >> >> Thanks a lot! > >I built the package for 3 rounds to check if there's something wrong. >I ran guix lint node-global-gradle-clean before submitting the patch. >It showed no warnings/corrections. I will do the same before submitting >subsequent versions of this patch. Thanks for mentioning this :) Great, thanks! > >Regards, >Dhruvin Gandhi
Hi, On Sat, 20 Mar 2021 at 06:58, Julien Lepiller <julien@lepiller.eu> wrote: > Le 20 mars 2021 04:51:41 GMT-04:00, Maxime Devos <maximedevos@telenet.be> a écrit : >>I prefer referring to the commit directly instead of by tag, as >>the commit is required for SWH fallback if the repo disappears. > > Sorry to contradict, but using a tag is much more readable and works > well with Software Heritage. At least, the linter seems to work in > that case. Have I missed something? Git tags should work with SWH, for both saving and falling back. Well, Git tags should not be an issue for SWH. If they are, it is an issue. I agree they are more readable and the current Guix packaging practise. On the other hand, they can lead to in-place replacement, whereas commits cannot. But it is not an issue about SWH, only caused by upstream bad practise. Cheers, simon