New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Read from MYMETA.{json,yml} #33
Conversation
} while ($path =~ s,^[^/]+/,,); | ||
push(@pmfiles, "$args{module}") | ||
} while ($path =~ s,^[^/]+/,,); # PP/LibYAML -> LibYAML | ||
push(@pmfiles, "$args{module}") # DateTime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coolo Shouldn't that be push(@pmfiles, "$args{module}.pm")
?
I added module names as comments to actually see what this code is trying to do.
And then I saw that for modules without ::
it would never add the .pm
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You ask me too much here - I didn't touch this code this decade. Sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. it was worth a try, could be that you remembered ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A problem with a v
version came up:
https://build.opensuse.org/request/show/960460
BuildRequires: perl(Parallel::ForkManager) >= v0.7.6
I need to find out why such versions weren't a problem before.
Actually we do already have modules where we require versions with a leading
That is something that cpanspec needs to handle in general, but this PR didn't introduce it, it just revealed the problem. I guess nobody noticed the wrong versions in the other modules so far. I created the ticket with a detailed explanation and what we could do: So the next step for cpanspec is to remove the leading I should do this in a seperate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you know what you're doing. I only have some coding style remarks but I suppose at this point they're not very important.
elsif ($meta->{license} =~ /^unknown$/i) { | ||
# do nothing, it's unknown and we know | ||
} | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And array with the patterns and their mappings would make sense. (So you could just loop through the array instead of having that many elsif
-blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. But I just moved this code around, and cannot make everything nicer :)
my $sh; | ||
if ($force) { | ||
rename($script, "$script~") if (-e $script); | ||
$sh = new FileHandle ">$script"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this way of calling the c'tor deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, see above :)
|
||
print $sh "#!/bin/sh\n\n" . "\@\@PERL_PROV\@\@ \"\$\@\" | sed -e '/^$filter_provides[0]\$/d'"; | ||
if (@filter_provides > 1) { | ||
for my $dep (@filter_provides[1 .. $#filter_provides]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If @filter_provides
isn't used anymore one you just shift
-away the first element and use a simple loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand that sentence. But I also have no idea what this code and the --filter-provides option is doing at all and I just moved it into a subroutine (so I can forget about it ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering the lack of tests, I second leaving it. Simplifying can backfire if you don't know the code
Issue: https://progress.opensuse.org/issues/92122 I put the code to read META.{json,yml} into subroutines, so I could reuse the code for MYMETA. Along the way I did many small refactorings. Now intrusive.pl and Intrusive.pm are not needed anymore.
Note: I will run this branch for a week or so on the autoupdate host to check if the updates look ok.
Issue: https://progress.opensuse.org/issues/92122
I put the code to read META.{json,yml} into subroutines, so I could reuse
the code for MYMETA.
Along the way I did many small refactorings.
Now intrusive.pl and Intrusive.pm are not needed anymore.