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

Reformatted dns::key and wrote tests for it #48

Merged
merged 6 commits into from
Apr 22, 2014

Conversation

danzilio
Copy link
Collaborator

No description provided.

@danzilio
Copy link
Collaborator Author

Commit 863bf3f will cause travis to start failing, but that's because it adds lint tests to the test suite. I think we need to keep this and start working on lint.

@solarkennedy
Copy link
Collaborator

Lets lint it up!

@danzilio
Copy link
Collaborator Author

👍

@solarkennedy
Copy link
Collaborator

Let me know if you want to divide the work, I could say, fix the errors for stuff in the record folder?

@danzilio
Copy link
Collaborator Author

Ok! I'll take everything under manifests/server.

@danzilio
Copy link
Collaborator Author

Commit 4018348 should make this PR green, but it breaks the dns::member class. I'm not entirely sure what the dns::member class was used for. If someone wants to export the dns::member defined type, why not just do it in a profile? This might need to be broken into a separate PR and would require a full version bump.

@danzilio
Copy link
Collaborator Author

...and, for that matter, why create a defined type to export? Why not just export the record inside the member? Maybe I should just refactor it that way so it doesn't break this functionality...

@solarkennedy
Copy link
Collaborator

Hmm. I don't know.... Yea I would just junk that member stuff.

I've pushed some cosmetic lint changes to master, feel free to merge or rebase.

danzilio added a commit that referenced this pull request Apr 22, 2014
Reformatted dns::key and wrote tests for it
@danzilio danzilio merged commit 175a920 into ajjahn:master Apr 22, 2014
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.

2 participants