Skip to content
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

Make module_name_rx extendable #6

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Make module_name_rx extendable #6

wants to merge 23 commits into from

Conversation

rurban
Copy link

@rurban rurban commented Jun 8, 2015

One might not accept the parser-based limitations of valid module names, e.g.
by creating packages in XS. E.g I use the int? type as type alias for (int | undef),
other might want to use unicode names.

This way you only need to asjust one variable: $Module::Runtime::module_name_rx

@karenetheridge
Copy link

Shouldn't the alteration of $Module::Runtime::module_name_rx be localized to the scope in which it's needed? otherwise, "interesting" things could happen elsewhere in the program.

I do support the desire to change the regex that is used, but I'm not sure if this is the way to do it.

@rurban
Copy link
Author

rurban commented Jun 9, 2015

Yes, you could localize your change to this package variable. This module
however just optionally picks such a change to the variable in the other
package.
On Jun 9, 2015 10:53, "Karen Etheridge" notifications@github.com wrote:

Shouldn't the alteration of $Module::Runtime::module_name_rx be localized
to the scope in which it's needed? otherwise, "interesting" things could
happen elsewhere in the program.

I do support the desire to change the regex that is used, but I'm not sure
if this is the way to do it.


Reply to this email directly or view it on GitHub
#6 (comment).

@ribasushi
Copy link

Given this patch has a very niche application and tangibly complicates the code (adding a hardcoded copy of a part of Module::Runtime, with an implicit logic fork depending if M::R has been loaded), I recommend holding off until doy/package-stash#15 is resolved, at which point the only relevant part of this PR would be the (expected to pass) addition to t/extension.t

Dist::Zilla::Plugin::MakeMaker 5.022 shifted to creating its file in the
FileGatherer phase, which is earlier than when PruneFiles runs, which
removed the small in-repo Makefile.PL so it can be replaced with the
generated version. Now we must prevent it from being gathered in the
first place.
This reverts commit 33a055a.

It's okay to have this as a requirement when RELEASE_TESTING=1, since we
normally release from a very recent perl and the variable is never set
otherwise.
in this commit, most plugins use the same config as before, so diffs are
kept to a minimum
      - quiet compiler warning (Jacques Germishuys, PR doy#2)
      - canonical repository moved to
        https://github.com/moose/Package-Stash=XS
...which necessitates removing trailing code so all the pod doesn't get
relocated.
..and the .h file will be updated in the repository after each release
@karenetheridge
Copy link

This distribution now lives at https://github.com/moose/Package-Stash-XS -- let's finish this up there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants