search

voxpupuli / puppet-pxe

Puppet module for deploying a PXE boot server
https://forge.puppet.com/puppet/pxe
51 stars 38 forks source link

allow to change syslinux archive url #20

Closed usp-rol closed 8 years ago

usp-rol commented 9 years ago

as with the 'baseurl' setting, the mirror to download can be changed too.

zachfi commented 8 years ago

@usp-rol Are you still interested in working on this?

usp-rol commented 8 years ago

@xaque208 We've forked before posting the pull request to satisfy our needs. What do you have in mind by "working on this"?

zachfi commented 8 years ago

@usp-rol No worries. If you're keeping your fork and not interested in getting the build passing and merged here, I can just close this out. I'd rather just not leave it open indefinitely if nobody is working on it.

usp-rol commented 8 years ago

@xaque208 Sure, we would be happy to see this request merged. You wrote that you didn't like the code style, basically I headed for this style. Can you give me some hints, what exactly you didn't like?

Lets close this request for now, I can file a new one later (with proper build status).

zachfi commented 8 years ago

Cool, thank you @usp-rol. This feature definitely seems like something this module should support, so I too would be happy to see it go in.

Personally, the only class I want to ever inherit from is the params class. Eventually I hope that this style goes away in favor of the 'data in modules' that Puppet4 brings. So I think what you have here is good, though I would make pxe::syslinux and pxe::memtest include the pxe class and simply reference those variables. Then users would be able to set the pxe::syslinux_url or whatever on the pxe class, and those variables would be referenced elsewhere in the module. This should keep it somewhat simpler for a user to implement, in my opinion.