Port to Distar #17

mohawk2 · 2018-01-28T05:43:48Z

Also includes as discussed a release-test to check existence of opt-dep .pod, and that it's newer than the .pm. The generating is done effectively at perl Makefile.PL-time and should probably instead be a build-time thing.

When I ran the Distar-enhanced make test, I got a xt/whitespace.t fail from a t/var/...something.pm. Probably needs a grep on that but the modules don't seem to offer an "exclude" option.

mohawk2 · 2018-01-28T06:28:35Z

The CI is failing, looks like it's missing deps like CAG and Test::Exception, probably because the .travis.yml needs updating. I will investigate.

haarg · 2018-01-28T12:31:03Z

Makefile.PL

+  };
+}
+delete $eumm_args{META_MERGE} if $eumm_version < 6.45_01;
+_move_to(\%eumm_args, 'CONFIGURE_REQUIRES', 'PREREQ_PM')


This doesn't accomplish much. It's too late for configure prereqs to be satisfied here. Deleting CONFIGURE_REQUIRES to silence the warning is the only useful thing that can be done.

Thanks, force-pushed!

haarg · 2018-01-28T12:32:45Z

Makefile.PL

+  if $eumm_version < 6.55_01;
+$eumm_args{NO_MYMETA} = 1
+  if $eumm_version >= 6.57_02 and $eumm_version < 6.57_07;
+_move_to(\%eumm_args, 'TEST_REQUIRES', 'PREREQ_PM')


This should move the prereqs to BUILD_REQUIRES and be moved before BUILD_REQUIRES gets conditionally moved to PREREQ_PM.

Thanks, force-pushed!

haarg · 2018-01-28T12:34:59Z

maint/Makefile.PL.include

+my @req_groups = keys %{ $optdeps->req_group_list };
+my @rdbms_groups = grep { /rdbms/ } @req_groups;
+my @other_groups = grep { !/rdbms/ } @req_groups;
+our (%dev_requires, %runtime_suggests);


I think this is a confusing way to communicate with the rest of the Makefile.PL.

I don't disagree, but it was the only way I could think of that fit with how Distar works. Do you have an alternative?

One option would be to return a hash from the script and do something like

my %dev_requires = -f 'META.yml' ? () : (do './maint/Makefile.PL.include' or die $@);

in Makefile.PL, but I'm not entirely sure I like that either.

I've done this instead as a hash-ref, allowing for future expansion by adding to the returned list.

haarg · 2018-01-28T12:41:12Z

Makefile.PL

+my $eumm_version = eval $ExtUtils::MakeMaker::VERSION;
+my %eumm_args = (
+    NAME => 'DBIx::Class::Schema::Loader',
+    AUTHOR => 'Caelum: Rafael Kitover <[email protected]>',


Redundant with Distar's author handing.

Thanks, force-pushed!

ilmari · 2018-01-29T13:02:02Z

Makefile.PL

+            x_MailingList => 'http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class',
+        },
+        no_index => {
+            directory => [qw(maint xt)],


PAUSE already doesn't index anything in inc, xt, and t, so I think we should either list t explicitly here for completeness, or only explicitly list maint for conciseness.

EUMM adds t and inc, so the list ends up sent to PAUSE as all 4. It is thus complete. The current code makes a META file with repetition.

Having checked the current CPAN META.yml, it in fact doesn't have repetition. However it did locally when I added t, so I left it out here, rationale as above.

ilmari · 2018-01-29T13:05:41Z

maint/Makefile.PL.include

+my @other_groups = grep { !/rdbms/ } @req_groups;
+our (%dev_requires, %runtime_suggests);
+%dev_requires = %{ $optdeps->modreq_list_for(\@other_groups) };
+%runtime_suggests = %{ $optdeps->modreq_list_for(\@rdbms_groups) };


We don't currently suggest any of the RDBMS-specific modules, and I'd like to keep actual behaviour changes to a minimum in this PR. If we want to further tweak the deps, that should be a separate discussion.

Agreed, changed.

ilmari · 2018-01-29T13:10:18Z

maint/Makefile.PL.include

+my @req_groups = keys %{ $optdeps->req_group_list };
+my @rdbms_groups = grep { /rdbms/ } @req_groups;
+my @other_groups = grep { !/rdbms/ } @req_groups;
+our (%dev_requires, %runtime_suggests);


One option would be to return a hash from the script and do something like

my %dev_requires = -f 'META.yml' ? () : (do './maint/Makefile.PL.include' or die $@);

in Makefile.PL, but I'm not entirely sure I like that either.

ilmari · 2018-01-29T13:12:38Z

Makefile.PL

+                web => 'https://github.com/dbsrgits/dbix-class-schema-loader',
+            },
+            x_IRC => 'irc://irc.perl.org/#dbix-class',
+            license => [ 'http://dev.perl.org/licenses/' ],


Doesn't EUMM automatically generate this from the LICENSE => 'perl' above?

Not as far as I can determine?

ilmari · 2018-01-29T13:15:26Z

Makefile.PL

+        'File::Temp'      => '0.16',
+        'File::Path'      => '2.07',
+    },
+    test => {TESTS => 't/*.t t/*/*.t'},


This needs to include t/*/*/*.t, since the backcompat tests are two directories deep. There aren't actually any files matching t/*/*.t, but I guess it doesn't hurt to include it for future-proofing.

Fixed. Counting is hard!

mohawk2 · 2018-01-30T08:25:41Z

The CI is still failing but now only on CLEANTEST=true. I will investigate this later, but in the meantime if reviewers can look over the CI changes and/or respond further on review points that will be very helpful.

ilmari · 2018-02-23T15:03:20Z

The Travis failures all seem to be due to dependencies failing to build or install, so not a blocker. I've adjusted things a bit and merged it as fb4432c, and pushed a dev release.

abraxxa · 2018-02-23T17:47:11Z

What‘s the reason for this change?

mohawk2 · 2018-02-24T16:35:37Z

@ilmari Let me know if further tweaks are required. I think it would be better for accountability if it were my name/id on fb4432c rather than yours. However, I guess I don't mind you getting the blame ;-)

mohawk2 · 2018-02-24T16:36:40Z

@abraxxa Moving off Module::Install, less complicated CI, easier release process.

mohawk2 added 2 commits January 27, 2018 23:04

add release test for optdep pod

7279266

add release test for optdep pod newer than pm

8ce5fe7

mohawk2 force-pushed the eumm branch from e7a1770 to f77e1a4 Compare January 28, 2018 05:59

haarg reviewed Jan 28, 2018

View reviewed changes

mohawk2 force-pushed the eumm branch from f77e1a4 to 6b4387a Compare January 28, 2018 18:30

ilmari reviewed Jan 29, 2018

View reviewed changes

port to Distar

7004d77

mohawk2 force-pushed the eumm branch from 6b4387a to 7004d77 Compare January 29, 2018 21:57

convert CI

2c5d1ef

mohawk2 force-pushed the eumm branch 5 times, most recently from 3f935ac to 47c8796 Compare February 10, 2018 23:28

workaround bizarre distro vs module confusion

e792a6d

mohawk2 force-pushed the eumm branch from 47c8796 to e792a6d Compare February 11, 2018 01:50

mohawk2 mentioned this pull request Feb 11, 2018

PERL_UNUSED_VAR(flags) is breaking on Perl 5.8.5 dankogai/p5-encode#129

Closed

ilmari closed this Feb 23, 2018

mohawk2 deleted the eumm branch July 15, 2018 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to Distar #17

Port to Distar #17

mohawk2 commented Jan 28, 2018

mohawk2 commented Jan 28, 2018

haarg Jan 28, 2018

mohawk2 Jan 28, 2018

haarg Jan 28, 2018

mohawk2 Jan 28, 2018

haarg Jan 28, 2018

mohawk2 Jan 28, 2018

ilmari Jan 29, 2018

mohawk2 Jan 29, 2018

haarg Jan 28, 2018

mohawk2 Jan 28, 2018

ilmari Jan 29, 2018

mohawk2 Jan 29, 2018

mohawk2 Jan 29, 2018

ilmari Jan 29, 2018

mohawk2 Jan 29, 2018

ilmari Jan 29, 2018

ilmari Jan 29, 2018

mohawk2 Jan 29, 2018

ilmari Jan 29, 2018

mohawk2 Jan 29, 2018

mohawk2 commented Jan 30, 2018

ilmari commented Feb 23, 2018

abraxxa commented Feb 23, 2018

mohawk2 commented Feb 24, 2018

mohawk2 commented Feb 24, 2018

Port to Distar #17

Port to Distar #17

Conversation

mohawk2 commented Jan 28, 2018

mohawk2 commented Jan 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohawk2 commented Jan 30, 2018

ilmari commented Feb 23, 2018

abraxxa commented Feb 23, 2018

mohawk2 commented Feb 24, 2018

mohawk2 commented Feb 24, 2018