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

Params refactor for future OS support with tests... on top of danzilio's refactor #34

Merged
merged 12 commits into from
Oct 23, 2013
Merged

Params refactor for future OS support with tests... on top of danzilio's refactor #34

merged 12 commits into from
Oct 23, 2013

Conversation

solarkennedy
Copy link
Collaborator

Sorry again for putting this on top of danzilios refactor. If you suck in his pull refactoring I will rebase to make mine simpler to read.

This adds a params class, similar to the Centos refactor, but no actual Centos support yet, just the framework. And tests!

I know this probably won't be accepted as is, but please let me know what works best for you. I don't really want to rebase back to your master though, as it lacks all the other testing infrastructure that danzilio put up.

@danzilio
Copy link
Collaborator

I haven't seen much movement on this repo. I think this module could be pretty awesome, but there are 12 pull requests sitting in the queue and the last pull request that was accepted was about a month ago. I hate to do this, but should we fork this module?

ajjahn added a commit that referenced this pull request Oct 23, 2013
Params refactor for future OS support with tests... on top of danzilio's refactor
@ajjahn ajjahn merged commit c0ad6e2 into ajjahn:master Oct 23, 2013
@ajjahn
Copy link
Owner

ajjahn commented Oct 23, 2013

@danzilio- You're right, there hasn't been much movement. I unfortunately have not been able to dedicate much time to maintaining/testing this module recently. You are welcome to fork it, but I'd much rather find one or two volunteers to assist in maintaining it. If you're interested let me know.

@danzilio
Copy link
Collaborator

I'm definitely interested!

@solarkennedy
Copy link
Collaborator Author

YES!

Can you add us as contributors and we can curate it?

If you ask me, the fewer forks = the better. (Just look at puppet-backuppc)

The only missing piece might be releases to the Forge. Forge releases are underrated if you ask me.

Are you still willing to periodically release things there? Or would you like someone else to takeover for that?

@solarkennedy solarkennedy deleted the params_refactor branch October 24, 2013 08:51
@ajjahn
Copy link
Owner

ajjahn commented Oct 24, 2013

I can still release things there, unless I can find a way to add a collaborator on forge. I think it makes since to keep it in the same spot.

@ajjahn
Copy link
Owner

ajjahn commented Oct 24, 2013

@danzilio @solarkennedy - by the way, you guys both have commit access now.

@danzilio
Copy link
Collaborator

Awesome! Thank you!

@ajjahn
Copy link
Owner

ajjahn commented Oct 24, 2013

No, thank you! So do you guys want to start by getting outstanding issues merged or closed? Then we can work toward a stable release of all the changes from your refactor, etc.

@solarkennedy
Copy link
Collaborator Author

Yay! I'll delete my fork!

Yea lets at least address/comment on the issues to get the rolling again, and yes a stable release to the forge that other people (work peeps) will feel comfortable using.

@danzilio
Copy link
Collaborator

I think that's a good idea. Triaging the pull requests is a good place to start. I've just started reading through them.

But I think the first thing to do, before merging some of these requests, is to set this repo up in Travis. The build status in the readme pulls from solarkennedy/puppet-dns. I don't think I can do this as a contributor...could you set that up?

Also, I think we should start requiring spec tests to be included in all future pull requests. I know this is a bit of a hard line, but this module is so well done, and has so much potential, I want to ensure that the code is of the highest quality.

@danzilio
Copy link
Collaborator

Also, along that line, I'm going to start writing some more tests.

@danzilio
Copy link
Collaborator

@solarkennedy I read your blog post about this module. I loved what you did with exporting the dns record (and I love how it could essentially be used to implement a light-weight dynamic dns). It's a really good example of how flexible this module is. I'm excited to see how we can enable even more innovative use of this module along those lines. We should add that pattern to the readme!

@ajjahn
Copy link
Owner

ajjahn commented Oct 24, 2013

@danzilio Travis should be set up. After it finishes the first build we should be able to update the badge. I agree on the tests.

Have a url for the blog post? Exporting dns records was my original vision for this module, I'd like to see how it is being used since it's been improved.

@danzilio
Copy link
Collaborator

Great! Here's @solarkennedy's blog post: https://xkyle.com/managing-dns-automatically-with-puppet/

Also, I would like to add lint and syntax checks to the standard Travis test suite...I'll work on that later today...

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.

3 participants