-
Notifications
You must be signed in to change notification settings - Fork 390
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
NEW FEATURE: IGNORE() (diff2 only) #2388
Conversation
a per-record basis using `IGNORE_NAME_DISABLE_SAFETY_CHECK`: | ||
|
||
``` | ||
// THIS NO LONGER WORKS! Use DISABLE_IGNORE_SAFETY_CHECK instead. |
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.
This is an great one to add on the changelog breaking chances section. This allows me, as a user, to determine the impact early on. Otherwise I update DNSControl and cannot run the commands before having to change the DNSControl domain configuration.
Having said that, what version number would you like to make this change under?
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.
it starts with diff2. so... v4.0.0 with "--diff2=false" as the temporary work-around?
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.
Yes, that could be a solution.
{% endhint %} | ||
|
||
* `IGNORE` is not tested with `D_EXTEND()` and may not work. |
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.
Question: is this still a problem? or is this traditional diff1 problem?
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 don't know. I just haven't tested it yet
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've tested it. It works fine.
You can override this error by adding the | ||
`IGNORE_NAME_DISABLE_SAFETY_CHECK` flag to the record. | ||
|
||
TXT('vpn', "this thing", IGNORE_NAME_DISABLE_SAFETY_CHECK) |
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.
Style: Same as others, missing the D()
and code highlighting.
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'm ok with not including D()
when it isn't intended to be a complete example.
Thanks for that! I've made a number of updates as a result. (It's amazing how a good preview finds typos!) |
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
I believe the code and docs are done. Any last comments? |
I'll go through it tomorrow. 👍 |
Revisions |
Looks good 👍 , just one minor point #2388 (comment) |
Thanks for the help! |
The IGNORE() function is now a generalized function that replaces the need for IGNORE_NAME() and IGNORE_TARGET(). It can ignore on label, type, and target. It supports globs.
IGNORE() is backwards compatible with the old IGNORE() function that is deprecated.
IGNORE_NAME() and IGNORE_TARGET() are shortcuts for equivalent IGNORE() calls.