-
Notifications
You must be signed in to change notification settings - Fork 400
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
DOCS: Explain ppreview/oldpreview changes #3135
Conversation
@cafferata Thoughts? |
| pre-v4.9 | sequential code | n/a | n/a | | ||
| v4.9 | sequential code | n/a | concurrent code | | ||
| v4.14 | concurrent code | serial code | concurrent code | | ||
| future | concurrent code | removed | concurrent code | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, why not, in accordance with Semantic Versioning, introduce a v5 where the old preview
/push
commands are removed? 🤔 This saves us the hassle of drawing up overviews. Of course we will include this in the next release notes as a noticeably ('breaking'?) change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by "drawing up overviews".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The (Markdown) tables as included in this GitHub pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see.
Here's what I'm trying to achieve:
- I want to encourage people to use the new code; and discourage people from using the old code.
- I want to remove the old code as soon as possible (maybe 2-3 months max).
What do you recommend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend:
-
Release v4.14 (<1 week)
- With a deprecation message within the
preview
andpush
commands.- Context: 'in de next release (
v4.15
) the parallel function (ppreview
andppush
) will be the default and so enabled for all providers. We encourage you to test the newppreview
andppush
commands for your own DNSControl set-up/configured providers.'
- Context: 'in de next release (
- Add a (similar) deprecation message to the
v4.14
release notes. - Send a mailing to the mailinglist with the release notes (deprecation message included).
- With a deprecation message within the
-
Release v4.15 (1 month)
- In release
v4.15
change the default behavior of thepreview
andpush
commands to the 'parallel version' (ppreview
andppush
) and while keeping the older serially version ofpreview
andpush
functionalities as fallback. Just like GitHub pull request FEATURE: parallel preview/push is now default #3132 does.
- In release
-
Release v5.0 (~3 months)
- In release
v5.00
remove the seriallypreview
andpush
commands. - Another nice removal for v5:
dnscontrol/commands/ppreviewPush.go
Line 178 in d6d50fc
printer.Println("WARNING: Please remove obsolete --diff2 flag. This will be an error in v5 or later. See https://github.com/StackExchange/dnscontrol/issues/2262")
- In release
I hope this helps you further in creating a release plan. If you have any questions, please let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent! Yes, that sounds good.
Should oldpreview
/oldpush
appear earlier i.e. in v4.14?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sequence looks good but I would think you can pull in the timelines if you want since v5 signals a major change and folks can decide when they want to roll it out internally,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should
oldpreview
/oldpush
appear earlier i.e. inv4.14
?
No, I would roll back this change #3132 in the main
branch and (re)apply it in a later release v4.15
. This gives the users another chance to prevent ignorance. After all, there will be (to be realised) a deprecation/log entry in the old preview
and push
commands.
Sequence looks good but I would think you can pull in the timelines if you want since v5 signals a major change and folks can decide when they want to roll it out internally,
That would certainly be possible. I would let the timeline depend mainly on the available time. @tlimoncelli himself spoke about 2/3 months where I assumed to prepare the various activities. In theory you could release v4.15
and v5.00
at the same time because it applies Semantic Versioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I propose we add oldpreview/oldpush as aliases for preview/push in v4.14. It dosn't hurt to make these commands available early.
-
The docs for oldpreview/oldpush should warn that these aliases are not subject to SemVer and should only be used with the understanding that they'll go away.
-
I disagree that we need to remove the old code in a major release (v5). Feature-wise, there's no breaking change between oldpreview and ppreview. The only breakage would be if someone uses oldpreview/oldpush in a script; my solution for that is in the previous paragraph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this to here: #3142
Fixes #3133