docs: remove obsolete CHANGELOG.rst file

[noelmcloughlin]

Last updated 2013 - not needed.

[johnkeates]

Yeah, that file is dead.

[johnkeates]

Commenting on my own comment: is there an official point at which rst was replaced by md as the preferred format? I know my own preference here, but I wonder if there is something I can point people to when I get that question.

[noelmcloughlin]

@myii @aboe76 do you know the guidance on REST vs MD

[saltstack-formulas-travis]

🎉 This PR is included in version 1.0.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[myii]

@noelmcloughlin There aren't any hard and fast rules at the moment but the general position at the current time is preferring RST, as outlined here. The ideal goal is to get Antora up and running -- a comparative view is here. Then a lot of the documentation can be centralised, pulling in from 300+ formula repos if necessary. That is written in AsciiDoc format, using Asciidoctor.

[johnkeates]

Ironically, the repo has both md and rst. I wonder where the rst preference originally came from, especially considering the amount of projects that are documented using md rather than rst. Is it just because of the Python's official support? Or simply initial preference within saltstack and now historical usage.

[myii]

@johnkeates From the little digging around I did, RST has been preferred across SaltStack Formulas from the early days. I surmise it was to maintain consistency with the main Salt documentation. A couple of short conversations we've had about this documentation issue:

• https://freenode.logbot.info/saltstack-formulas/20190302#c2034518
• https://freenode.logbot.info/saltstack-formulas/20190304#c2036441

[johnkeates]

@myii Most of the discussions make sense, and it seems to be pretty recent having the same thoughts. While RST (and Asciidoc) are technically superior to Markdown, I do see MD more often read and written than the others on the hipster side of development (slack, github, most web/electron development systems), while the technically better systems are more on the hardcore/real-deal side of projects (i.e. those that do their main work on mailing lists, using patches or gitweb etc). Perhaps it's a culture thing or a visible/invisible thing like an iceberg having MD visible on top.

Since RST is Python's default inline-ascii documentation format that seems a good fit, but for self-hosted docs, asciidoc makes more sense, especially considering maintainability. While furthering integration with salt and formulas in diverse private devops shops, I've noticed most of them reading from the generated salt docs and writing to their internal documentation systems (mostly confluence, some mediawiki, and a few others).

At this point this writing is becoming too much of a brain dump and not really suitable for this pull thread; I guess I could condense it into: MD is more popular for contributing (very low friction) but since documentation is often read much more than written, a better system (especially inside an ecosystem like saltstack-formulas) makes more sense, even if that means that some contributors might find it harder to build on. I guess I've made the conversation moot :)

[myii]

@johnkeates We sound like we're all on the same page, essentially.

By the way, I've had an uneasy feeling that some users may get caught out by the nginx.ng change. Would it be useful to have a temporary banner in the README, for a month or so until most have converted to the new system?

CC: @daks @noelmcloughlin @n-rodriguez.

[noelmcloughlin]

TL;DR: no, wait a little
Best practice is to depreciate an interface - i.e. xxx.ng.yyy redirecting to xxx.yyyand stop documenting the old interface (and maybe state ng interface is depreciated). In this case the interface is gone away - make a decision based on feedback - if feedback is positive, continue this approach, if feedback is too negative see if we can depreciate ng before getting rid of it in future.

[myii]

@noelmcloughlin nginx.ng has already gone, as of v1.0.0, after the merge of #232. So the idea was to put a banner explaining that the last version with nginx.ng (and the old nginx) was v0.56.1 -- i.e. pin to that tag if still required.

[n-rodriguez]

Would it be useful to have a temporary banner in the README, for a month or so until most have converted to the new system?

I agree. I get caught by it 2 hours ago ^^. I forgot that the .ng was removed. Fortunately I ran the state with test=true 👍

Note: I say that because changing the states was easy. But don't forget to update your pillars

[johnkeates]

Chiming in on the nginx.ng removal:

I agree with both; while I don't know of a neat way to generate standard deprecation warnings from salt formulas (as in, consistent across multiple formulas as they will all be namespaced) it would be neat to have an alias with deprecation warning first, and then update to a version that errors out with a removed fatal error. But I'm afraid we aren't at that level yet.

On the documentation side, adding a banner or warning is definitely a good idea.

Looking over the fence at other tools, Terraform handles this in the deprecation->removal->fatal chain on a per-release basis. This eases transitions and means users will be informed, warned and directed towards a solution. This is what is generally done for stable API products as well, i.e. the Linux kernel, macOS and perhaps even Windows (but I don't know, I try to stay away from that one on the API level).

Wat we can do now:

• Add a warning about the changed state names
• Add instructions on pillar changes

Wat we may want to figure out for similar changes:

• Can we setup a consistent runtime deprecation warning system
• Can we automatically alias or transition states
• Can we chain pillars (probably not because you'd have to copy a pillar to another pillar after they have been rendered) to alias old pillars to new pillar names and warn the user about it

[myii]

So we've got the banner in place but I reckon we can go one step further based on a WIP PR we've been discussing in the template-formula. The implementation here won't involve any of the .yaml files but will need to be included in all of the main states listed on the README. Basically, we block further execution if nginx:ng is detected. @johnkeates, what do you think?

[johnkeates]

We can block execution but we should throw a descriptive message for the user as to why. Any way to colorise or otherwise make such a message stand out? Most of the stuff I've tried either gets lost in automation (i.e. when only showing changed in output, or setting it to terse) or is logged but not displayed (i.e. python exceptions). @myii What is your take on this?

Edit: I was specifically thinking of test.* like it is used in that WIP PR, it works great as long as full output is on or it's not hidden in an automation pipeline. Any ideas on how to get test.* or something similar to output a "better" message?

[myii]

@johnkeates The method I'm suggesting will prevent the execution of any of the deprecated states with a hard failure. The message there can be customised accordingly. I've noticed that none of the states overlap with the old nginx states, so we can recreate dummy files that simply include the unsupported state. This can also be done for the "old" nginx.ng states. Essentially, the endpoints will be available again but each of them won't do anything other than giving an explanation of what has changed.

[myii]

@johnkeates So this is the only state that will "run" for all deprecated states (failing, red text):

minion:
----------
          ID: nginx-deprecated-test-fail
    Function: test.fail_without_changes
        Name: 

################################################################################
#                                                                              #
#                   WARNING: BREAKING CHANGES SINCE `v1.0.0`                   #
#                                                                              #
################################################################################
#                                                                              #
# Prior to `v1.0.0`, this formula provided two methods for managing NGINX; the #
# old method under `nginx` and the new method under `nginx.ng`. The old method #
# has now been removed and `nginx.ng` has been promoted to be `nginx` in its   #
# place.                                                                       #
#                                                                              #
# If you are not in a position to migrate, please pin your repo to the final   #
# release tag before `v1.0.0`, i.e. `v0.56.1`.                                 #
#                                                                              #
# To migrate from `nginx.ng`, simply modify your pillar to promote the entire  #
# section under `nginx:ng` so that it is under `nginx` instead. So with the    #
# editor of your choice, highlight the entire section and then unindent one    #
# level. Finish by removing the `ng:` line.                                    #
#                                                                              #
# To migrate from the old `nginx`, first convert to `nginx.ng` under `v0.56.1` #
# and then follow the steps laid out in the paragraph directly above.          #
#                                                                              #
################################################################################

      Result: False
     Comment: Failure!

[johnkeates]

I suppose it’s the best option we have right now. For future deprecations we should add a messenger state that warns the user ahead of upcoming releases, we could tack this on the same way as we do now, except the existing state will still be there and run.

[myii]

@johnkeates Initial testing is looking good. I'm a bit short on time but I'm going to try to push this out into a PR in a few minutes, so that it can at least be considered for inclusion, if not merged outright.

[myii]

@johnkeates So I've pushed PR #236 as the temporary workaround. We need to make sure that the tests pass first. It would be good if others could test these changes before merging.

[johnkeates]

@myii was the middle of the night here so I kinda went to sleep, couldn't merge it ;-) But #236 seems to be resolved.