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

Use helpers.shema.Provisoner in chef provisioner #12033

Closed
wants to merge 2 commits into from
Closed

Use helpers.shema.Provisoner in chef provisioner #12033

wants to merge 2 commits into from

Conversation

VladRassokhin
Copy link
Contributor

@VladRassokhin VladRassokhin commented Feb 17, 2017

Also ValidateFunc introduced for extended provisioner data validation.

This PR is rebase of #10422 on top of master after #10934 being merged.

@VladRassokhin
Copy link
Contributor Author

@mitchellh Please take a look. I've rebased my branch after your changes in provisioners

1. Migrate `chef` provisioner to `schema.Provisioner`:

 * `chef.Provisioner` structure was renamed to `ProvisionerS`and  now it's decoded from `schema.ResourceData` instead of `terraform.ResourceConfig` using simple copy-paste-based solution;
 * Added simple schema without any validation yet.

 2. Support `ValidateFunc` validate function : implemented in `file` and `chef` provisioners.
@VladRassokhin
Copy link
Contributor Author

@paddycarver @mitchellh Sorry for disturbing but could anyone merge/comment on this PR ❓

@svanharmelen
Copy link
Contributor

@VladRassokhin I will review this one, but it will take a few more days until I am behind a desk again (and this PR is a bit too big to review on my phone 😉) So unless someone else steps in, I'll make sure to make some time for this one in the coming week.

@svanharmelen
Copy link
Contributor

svanharmelen commented May 19, 2017

@VladRassokhin I spent a reasonable amount of time on this one today... I started with a normal review, but it soon became a challenge to mark and explain my findings/concerns.

So I thought the fasted way forward was to created a new branch based on your work and add a single commit tweaking what was (IMO) needed to get this ready to be merged.

Overall I didn't really change any fundamental stuff. There was one real issue with the schema (with the fields that contained an Elem) and other then that I just did some refactoring and cleaning (e.g. removed the deprecated fields).

So thank you very much for all your work and my apologies for the initial leak of response! I'll be closing this PR as soon as the new one is merged (which will hopefully be today).

@svanharmelen
Copy link
Contributor

OK, so I'm going to close this one right away as I need an additional LGTM on the new PR since I now added code myself as well 😉

So to not cause any confusion with these almost similar PR's, I'll close this one and refer you to the new PR #14681.

Thanks again!!

@VladRassokhin VladRassokhin deleted the f-provisioners-api-master branch June 7, 2017 00:23
@ghost
Copy link

ghost commented Apr 11, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants