search

saltstack-formulas / dhcpd-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
8 stars 56 forks source link

Rewrite of dhcpd-formula #26

Open mthibaut opened 6 years ago

mthibaut commented 6 years ago

Hi,

I am wondering what you will accept. I don't want my work to go to waste... If I do the work, will you accept it? Also: shall I put my "new" stuff under a new pillar like the nginx formula, i.e. like this:

dhcpd:
    ng:
        authoritative: true

This would stop any name clashing between the old and the new stanzas. But it would make things awkward, perhaps, since IPv6 isn't even present in the old style.

Any thoughts?

javierbertoli commented 6 years ago

Hi @mthibaut, first thing first, I'd say your work will be welcome, I don't see a reason to consider it a wasted effort. I think that a (major) rewrite will mean a lot of consideration before being approved and merged. But I'd say it will be definitely considered.

Now, going to the work you're doing, I think that merging tests (#25) would be the first thing to do (I'll try to do it between today and tomorrow), and the move on with the refactoring.

On that issue, I personally prefer:

Just my 2c.

mthibaut commented 6 years ago

Thanks for your comments!

a refactoring that does not imply a ng, but rather modify/improve the formula as is.

As you can see in the ntp formula and in PR #18 there is no way to fix the issues in dhcpd-formula except by starting over from scratch. We would end up with really weird configs containing an impossible-to-understand mish mash of underscores and dashes. Putting things inside "ng" is the best of both worlds, allowing people to use the old syntax while providing a more complete and functional "new" syntax.

that the refactoring consider, as much as possible, backward compatibility, at least in states and pillar formats

The current refactoring has the same top level structure, it just introduces new macros and templates to fill in the blanks. If we want to fix some of the issues in dhcpd-formula that can be fixed, we will automatically converge slowly anyway -- with the only exception being these macros and templates. The convergence would be mainly for OS families and the like. IPv6 as well I think.

if possible, you can consider submitting different PRs, with smaller changes, (ie, just adding IPv6 support) so they can be evaluated and merged quicker: always consider that most of us do this on our spare/free time, and therefore reviewing big PRs is definitly going to take more time.

I do understand, I have the same issue :). I can give it a go for IPv6 though this introduces a batch of new statements. But for the refactoring this will be difficult; there I would rather see an agreement on the approach or a suggestion of a different approach. Would you want some examples of why we need the stuff in PR #18?

IliyanIliev1 commented 5 years ago

Guys, does anybody from you knows how should be written pillar to add option between class {}

baby-gnu commented 5 years ago

Hello.

I just created the pull request #31 which add travic CI and enable semantic-release.

When it will be merged, I'll start to convert the existing formula to template-formula standard without changing how it works (not like #18).

Regards.

myii commented 5 years ago

@mthibaut Another benefit of implementing semantic-release is that it allows for major rewrites, as we've already done with some of the ng formulas. So if you're still willing to make a large contribution, we'd be better placed to accommodate for it after that. Of course, it would be good if the plans can be agreed first between the major contributors, to ensure no-one wastes any time.