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 Inline::Module::MakeMaker be more OO #8

Closed
ingydotnet opened this issue Nov 18, 2014 · 15 comments
Closed

Make Inline::Module::MakeMaker be more OO #8

ingydotnet opened this issue Nov 18, 2014 · 15 comments

Comments

@ingydotnet
Copy link
Owner

FixMakefile is an export. It should create a module for state and then call:

$self->fix_makefile;
@Leont
Copy link
Collaborator

Leont commented Nov 18, 2014

If it's a proper MakeMaker extension, you don't need to do anything other than loading it.

@ingydotnet
Copy link
Owner Author

It's not a MakeMaker extension. Should it be? Can you give me an example EUMM extension that fixes up the Makefile?

FWIW, I'm not tied to any specific implementation. I've already refactored a few times. Willing to keep changing things until it is right.

@Leont
Copy link
Collaborator

Leont commented Nov 19, 2014

It's not a MakeMaker extension. Should it be? Can you give me an example EUMM extension that fixes up the Makefile?

Why would you want to fix up the Makefile? Why not write a correct one the first time? Postambles are used by hundreds of modules, that way you don't need to fix anything. The pure_all additions can be moved trivially, the destdir addition would require an additional action that depends on create_distdir and is a dependency of distdir (single colon does allow you to add dependencies, even if it doesn't allow you to add actions).

@ingydotnet
Copy link
Owner Author

Can you give me an example module that adds a postamble?

I'll check if I can do it all in a postamble.

@mohawk2
Copy link
Contributor

mohawk2 commented Nov 19, 2014

Module with a postamble: https://metacpan.org/source/ETJ/Gimp-2.31/Makefile.PL

@ingydotnet
Copy link
Owner Author

I meant that I want to see a module that one would use in a Makefile.PL that adds a postamble. (Not a module dist with a Makefile.PL that adds a postamble directly).

@karenetheridge
Copy link
Collaborator

@ingy I asked you a few days ago why you weren't using a postamble, showing
you an example right in the [MakeMaker::Awesome] documentation :D

On Tue, Nov 18, 2014 at 6:04 PM, mohawk2 notifications@github.com wrote:

Module with a postamble:
https://metacpan.org/source/ETJ/Gimp-2.31/Makefile.PL


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

@mohawk2
Copy link
Contributor

mohawk2 commented Nov 19, 2014

@ingydotnet
Copy link
Owner Author

FWIW, a postamble is just appending stuff to a Makefile. It's not that big of a deal. All I'm doing now is appending to the Makefile (and changing 'distdir :' to 'distdir ::'). I'll be glad to use a postamble technique if it's not a pita, but this isn't really buying us much.

@karenetheridge I can't locate your example. URL please?

@daoswald
Copy link
Collaborator

@Leont
Copy link
Collaborator

Leont commented Nov 19, 2014

My own example would be this.

@ingydotnet
Copy link
Owner Author

Let me change the tune here. We are all not on the same page. Here is the current Makefile.PL API:

https://metacpan.org/pod/distribution/Inline-Module/lib/Inline/Module/Tutorial.pod#The-Makefile.PL

Whatever we settle on internally, I don't want that API to become worse. It's a clean API, and I'm happy to make it cleaner, but not happy to make it worse.

All I've suggested so far is to change use Inline::Module::MakeMaker to use Inline::Module, which is cleaner.

I just looked at @Leont 's example, and it looks worth checking out, but I'm not sure it will work out. @Leont , can one use several modules like yours to add several postambles?

@ingydotnet
Copy link
Owner Author

@Leont , I got some time to look at your module, and while it is reasonable, it doesn't allow for multiple modules of that style. In other words, if I did it that way, people couldn't use both of our modules.

That is one of the reasons I went to the current FixMakefile thing. It plays well with others. The other reason is that I can't do all I need in a postamble. See '::' thing above.

@ingydotnet
Copy link
Owner Author

I:M:MM is OO now. So closing this. People can continue to comment on the
postamble stuff though.

@Leont
Copy link
Collaborator

Leont commented Nov 19, 2014

I got some time to look at your module, and while it is reasonable, it doesn't allow for multiple modules of that style. In other words, if I did it that way, people couldn't use both of our modules.

You can, as described in the docs. There is a -global option that installs itself as the global postamble, and a non-global one that you can use in your own postamble (and actually the whole point of the thing is that you usually don't need your own postamble, but that's another discussion).

The other reason is that I can't do all I need in a postamble. See '::' thing above.

Which is nonsense.

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

No branches or pull requests

5 participants