feat(ng): promote NG formula

[sticky-note]

This PR include:

• Promotion of NG formula
• Introduction of recommended tplroot var
• {%- consistency

PS: I've not yet tested these changes.
I began this work to have feedback first.
Ping @myii @n-rodriguez @aboe76

[myii]

@sticky-note In terms of the deprecation messages in cf05b73, if you look at https://github.com/saltstack-formulas/nginx-formula/pull/236/files, you'll see that we have to add these to the states that we're deprecating, i.e. the non-ng states that are going to be removed.

[myii]

Copying across our earlier conversation from Slack:

-	-	-
00:28	zabiteus93	Hi folks, can someone help me with this ? #183
00:33	myii	@​zabiteus93 Apologies, very busy time at the moment. I saw the PR and was meaning to give you some initial feedback there.
00:34	myii	Now, with ng conversions, we've got to do this over stages. We went a little too fast with the nginx-formula, with no time to give warnings beforehand.
00:35	myii	Since then, we put the warnings in retrospectively but that really should have been the first stage.
00:35	myii	So taking your PR in general, that's a bit further down the line -- we have to build towards it.
00:36	myii	What we need first is a deprecation warning in the next "release" of php-formula. It can use a similar method to the one used by nginx-formula.
00:38	myii	This PR is the basis for that, which links to the others: saltstack-formulas/nginx-formula#236.
00:40	myii	This time around, since we're only warning at first, change both test.fail_without_changes and failhard: True -- these will be reintroduced once the deprecation has taken place.
00:41	myii	And the message should be modified likewise.
00:41	myii	All of this requires another PR first. Are you ready to do that @​zabiteus93?
02:38	zabiteus93	I have to dig into some regression first and yes, I'm really enthusiast to do that
02:39	zabiteus93	If somenone can help me with some test, it'll be really aspreciated
05:11	zabiteus93	deprecated state done : //github.com/saltstack-formulas/nginx-formula/pull/236Debian 9 travis check fails, don't know why yet

[myii]

@sticky-note As I mentioned in Slack, it is better if we merge the deprecations warnings first, in a separate PR. We want to release a version that starts warning people to get ready to move across to php:ng. That way, we don't have too much going on in this PR as well. It is already too much to review, with over 200+ files. It would be great if we could somehow do this in smaller stages.

Any suggestions @n-rodriguez, @aboe76, @daks?

[johnkeates]

Same as with nginx:ng (but that went wrong IIRC)

[daks]

In fact, nginx NG promotion has been done quite quickly and we have learnt from it, so I'm also in favour of doing a release to add the deprecation warnings.

[n-rodriguez]

I'm also in favour of doing a release to add the deprecation warnings.

👍

[myii]

First merge a PR with deprecation warnings with a major release, then Promote ng with another major release ?

@sticky-note That's right but the deprecation warnings PR doesn't have to be a major release. A couple of points about what needs to be done:

1. Each state file under php (non-ng) needs a deprecation warning included, explaining that these files will be removed in upcoming v1.0.0.
2. Each state file under php.ng needs an explanatory message included, informing that these files will be promoted to php in v1.0.0 (i.e. that the .ng will be dropped).

Obviously, all of these using an include at the top of the file.

[sticky-note]

@myii I have some time to edit this PR
I had to merge master into this branch because of multiple rebases
How can I get rid of the merge commit ?
We need to modify the warning messages too
Do we need also to leave the ng folder with all of the older .sls with an

include:
  - php.ng.error

or something ?

[myii]

Just merged #167, so a little more rebasing here @sticky-note, sorry!

[myii]

@sticky-note Hold on, I'll try to get you something patched up.

[myii]

@sticky-note OK, I've patched things up and put it together in one commit on top of the latest upstream here: myii@7fbb40b. Have a look and see how that compares with the work you have done.

The problem here is that too many changes are being made at the same time, which makes it difficult for git to follow what's going on. For example, when patching this again, it kept tripping up on:

• All of the {% which have been fixed to {%-
• The introduction of tplroot via. {%- set tplroot = tpldir.split('/')[0] %}

While these are useful changes (to make eventually), these should be left for a subsequent PR, so that this one can be applied cleanly first. In fact, it may still be the sensible option to do that, so that we can be completely confident that these changes are sound. We can always compare with the linked branch, to reintroduce the extra changes in a subsequent PR.

[sticky-note]

#183 (comment)
No problem

• I've removed php.ng.deprecated.sls andphp.deprecated.sls from pre-commit_semantic-release.sh and release.config as well
• Copied the Readme.rst message from nginx fomula

Seems to be correct but Needs real tests

Do we need to leave ng state with an error.sls include or something ?

[sticky-note]

@myii, I've rebased with master
Not sure if pillar.example is correct

[myii]

I've rebased with master

@sticky-note Sorry again for the trouble. Let's hope this is the last time.

Not sure if pillar.example is correct

It appears to be fine. What do you think is incorrect?

[sticky-note]

@myii

It appears to be fine. What do you think is incorrect?

I've not digged in details to the very last modifications.
I've just trusted the mergetool with some fixes concerning ng
If you think it is correct, so go to finalize this PR.
What to to next ?

[myii]

What to to next ?

@sticky-note That's all from my end and hopefully from your end. We're just waiting for the final confirmations before this can be merged. Looking above, it's what @aboe76 has mentioned:

Can somebody confirm this works on Centos/RedHat or Fedora?

@philpep @arthurlogilab Can either of you do that?

[sticky-note]

Want to test it on CentOS with vagrant to save some time. Is it ok @myii ?

[myii]

Want to test it on CentOS with vagrant to save some time. Is it ok @myii ?

Sure, it just needs to be confirmed and then we should be able to proceed. Is that OK, @aboe76?

[sticky-note]

@myii @aboe76 States for php-fpm, fpm-pools and around twenty php modules, and composer installation with scl repo enabled and php_version : 72 are confirmed to work on a CentOS 7 vagrant box

[myii]

Thanks @sticky-note, I'm happy for this to be merged. Any final comments @aboe76, @n-rodriguez @daks @johnkeates?

[aboe76]

Lgtm

[n-rodriguez]

Lgtm

same here 👍

[myii]

Congratulations @sticky-note, it's merged -- thanks for your hard work and patience!

Appreciate all of the feedback and reviews from everyone else.

[saltstack-formulas-travis]

🎉 This PR is included in version 0.41.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[myii]

@sticky-note The commit message fixes must have got lost during all of the rebases, so it didn't trigger a v1.0.0. See myii@5aade39:

BREAKING CHANGE: all previous `php` based configurations must be reviewed;
`php.ng` usage must be promoted to `php` and any uses of the original
`php` will have to be converted.

Since BREAKING CHANGES (extra S) was used instead, that's why this happened. Don't worry, I'll add a commit now to fix this.

[myii]

Resolved in 57b37dd. Welcome to v1.0.0!