staged-recipes
staged-recipes copied to clipboard
Added recipe for Perl module Moo and its deps
This PR adds recipes for Perl module Moo and its dependencies. The following recipes are provided:
perl-class-method-modifiers
perl-moo
perl-role-tiny
perl-sub-quote
perl-test-fatal
Checklist
- [x] Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
- [x] License file is packaged (see here for an example).
- [x] Source is from official source.
- [x] Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
- [x] If static libraries are linked in, the license of the static library is packaged.
- [x] Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
- [x] Build number is 0.
- [x] A tarball (
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details). - [x] GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
- [x] When in trouble, please check our knowledge base documentation before pinging a team.
Hi! This is the friendly automated conda-forge-linting service.
I wanted to let you know that I linted all conda-recipes in your PR (recipes/perl-class-method-modifiers
, recipes/perl-moo
, recipes/perl-role-tiny
, recipes/perl-sub-quote
, recipes/perl-test-fatal
, recipes/perl-test-needs
) and found some lint.
Here's what I've got...
For recipes/perl-class-method-modifiers:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-moo:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-role-tiny:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-sub-quote:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-test-fatal:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-test-needs:
- Feedstock with the same name exists in conda-forge.
Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipes/perl-class-method-modifiers
, recipes/perl-moo
, recipes/perl-role-tiny
, recipes/perl-sub-quote
, recipes/perl-test-fatal
) and found it was in an excellent condition.
I do have some suggestions for making it better though...
For recipes/perl-class-method-modifiers:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-moo:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-role-tiny:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-sub-quote:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
For recipes/perl-test-fatal:
- Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
@conda-forge/bioconda-recipes, I'm porting the recipes mentioned above to CondaForge if there are no objections from your side.
@conda-forge/help-perl please review!
@cbrueffer I added you as maintainer to all recipes above, is this okay for you? Also, I would appreciate your review ;-) Thank you so much!
@cbrueffer If you don't have time to review right now, that's ok, but could you please give your consent (or not!) for being a maintainer? Then someone else could also do the review ... Thank you!
Yes, I agree to be a maintainer. I'll try to look at this later today or tomorrow.
Technically the recipes look OK (see questions on the commented out dependencies), I've added some comments on how to improve them on a style level to improve consistency. I've only made those on the first recipe, but they apply to all recipes in the PR. Here's an example of a (at least in my view) well-formatted recipe: https://github.com/conda-forge/perl-scalar-list-utils-feedstock/blob/main/recipe/meta.yaml
Generally PRs with only one recipe are much more motivating to review than large PRs; something to keep in mind to ensure quick reviews.
Technically the recipes look OK (see questions on the commented out dependencies), I've added some comments on how to improve them on a style level to improve consistency. I've only made those on the first recipe, but they apply to all recipes in the PR. Here's an example of a (at least in my view) well-formatted recipe: https://github.com/conda-forge/perl-scalar-list-utils-feedstock/blob/main/recipe/meta.yaml
Thanks for for your feedback, @cbrueffer. Unfortunately I don't see any comments in the files section, am I blind or did you miss to submit something? Since you explicitly mentioned the commented out deps, I comment on this: they were generated by conda skeleton
and I assume the background is that it is assumed that all Perl recipes define themselves as weak run-export, i.e. when such packages are used as a build
-time dep, they are automatically assumed to be a run
-time dep as well. Therefore, they are not explicitly required there. I now removed them completely and everything still works nicely. I hope that we are ready to merge now!
Generally PRs with only one recipe are much more motivating to review than large PRs; something to keep in mind to ensure quick reviews.
Okay, well, it is much more efficient to prepare a bunch of recipes when the actual target recipe has dependencies which are not yet provided by conda-forge
. I could factor out individual recipes, but that's quite some effort (testing each of them again, single recipe tests take much longer in total than a bulk of recipes in one build). I have another recipe in preparation that ports Moose to conda-forge
and it has some 30 recipes in total. Do you really think that 30 individual PRs would be easier to review? Also it would take very long until all dependencies are finally available for the recipes closer to the root of the dependency tree. What do you think?
Seems I left the comments pending and forgot to actually submit the review, sorry about that. Yeah, my question was around the commented out dependencies; I think simply removing them is better.
As for the size of PRs, yeah, 30 dependencies is a lot. My point was just that the bigger the PR, the more motivation needed to actually dig through it (at least for me, YMMV). If the recipes are in good shape it removes some of the pain though; how about at least chunking it into 3 x 10? You can also add the build instructions directly into meta.yaml
, that removes 2/3 of the files to review directly.
@cbrueffer Fixed the issues you mentioned. Good catch with the shebang lines, I removed them when filtering out comments -- stupid me. Concerning the idea of moving the build script into meta.yaml
: I will look into that; I have to check the module builder in my scripts (ExtUtils::MakeMaker or Module::Build) to get the code right, but could add that to my scripts. I would do that for future recipes, though. Another thought: what about Windows support? Currently, we don't have a Perl package for Windows, but if that becomes available again at some point in the future, having bothbld.bat
and build.sh
in place would make supporting it easier, wouldn't it?
Could we merge this now? I need some of the packages as deps for other recipes ;-) Thank you!
The bld.bat
files still have EOF whitespace but I'm merging it anyway now. Regarding Windows, I think the inline build instructions should work regardless. There are cases where something additional needs to be done during the build, but those could always have separate build files. I'm happy to merge Windows builds, but personally I don't care about it; my main concern is moving Perl out of Bioconda.