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

Initial stab at issue/1 #2

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Initial stab at issue/1 #2

wants to merge 8 commits into from

Conversation

jcpunk
Copy link

@jcpunk jcpunk commented Apr 13, 2015

Odds are this needs some work, but I think I got the basics working

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8910f03 on jcpunk:issue-1 into * on hercules-team:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8910f03 on jcpunk:issue-1 into * on hercules-team:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8910f03 on jcpunk:issue-1 into * on hercules-team:master*.

identity = lambda { |x| x }
[
[
/^((\S+)\s+of\s+(.+))$/,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I'm right... but looking over in ap_postgresql it seems like maybe you could take a different approach here?

https://github.com/hercules-team/augeasproviders_postgresql/blob/master/lib/puppet/type/pg_hba.rb

Is namevar needed? or just that thigns that make it up.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My ruby is "super bad". I've never learned the language and am mostly programming by pattern.

It looks like I'm doing similar things to what the pg_hba type is doing....

Alas namevar was created so I didn't have to change the other code. Puppet was treating ':name' as the namevar and when splitting the complex name into attribs the use of the directive name as ':name' was causing duplicate resource definitions.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3f15a6c on jcpunk:issue-1 into * on hercules-team:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling db10bd7 on jcpunk:issue-1 into * on hercules-team:master*.

@@ -26,6 +26,14 @@ def insync?(is)
end
end

newparam(:namevar) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather user name than namevar for that.

I know this might change the behavior, so I'd suggest something like this:

  • use :name as the default namevar
  • add a :directive parameter for the directive name
  • feed both :name and :directive in self.title_patterns with respectively the full match and the directive only
  • have :directive default to self[:name] for compatibility purposes

This should make it backwards-compatible, and provider a nicer interface imo.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3008981 on jcpunk:issue-1 into * on hercules-team:master*.

@@ -89,6 +95,7 @@ Type documentation can be generated with `puppet doc -r type` or viewed on the

The `SetEnv` directive is not unique per scope: the first arg identifies the entry we want to update, and needs to be taken into account. For this reason, we set `args_params` to `1`.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That newline seems to have been added by mistake.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling ac2fda2 on jcpunk:issue-1 into * on hercules-team:master*.

@vox-pupuli-tasks
Copy link

Dear @jcpunk, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@vox-pupuli-tasks
Copy link

Dear @jcpunk, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants