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

DOCS: Trailing commas are no longer an issue #3248

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

cafferata
Copy link
Collaborator

This pull request updates the documentation to reflect changes in v4.15.0, where END is no longer required in code examples, and trailing commas are no longer an issue. 🥳

Fixes #3245

@@ -9,70 +9,6 @@ Solution: Use a "builder" to construct it for you.
* [M365_BUILDER](language-reference/domain-modifiers/M365_BUILDER.md)
* [SPF_BUILDER](language-reference/domain-modifiers/SPF_BUILDER.md)

# Trailing commas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem: Someone sees an old dnsconfig.js with END and can't find it in the docs.

Suggestion:

END

You may see D() statements written with END at the end. For example:

{% code title="dnsconfig.js" %}

D("example.com", REG_MY_PROVIDER, DnsProvider(DSP_MY_PROVIDER),
  A("foo", "1.2.3.4"),
END);

{% endcode %}

As of DNSControl v4.15.0 these END statements can be removed. END is a no-op which was added for reasons that no longer matter. They can be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Tom, for your feedback! I’ve incorporated it into this commit. You can preview the updated documentation here.

@jauderho
Copy link
Contributor

So is it better to not have trailing commas going forward?

@tlimoncelli
Copy link
Contributor

So is it better to not have trailing commas going forward?

Both work. I encourage people to include the trailing comma because it makes "git diff" easier to read.

Now that both work, there's no need for the "END" hack to force it to work.

@tlimoncelli
Copy link
Contributor

Ready for merge?

@jauderho
Copy link
Contributor

BTW, the last I checked, the output generated by dnscontrol write-types still outputs END.

@tlimoncelli
Copy link
Contributor

BTW, the last I checked, the output generated by dnscontrol write-types still outputs END.

Looks like it only outputs the description of the END command itself:

$ git co cafferata-docs/code-tricks-end
Already on 'cafferata-docs/code-tricks-end'
$ go install
$ dnscontrol write-types
Successfully wrote types-dnscontrol.d.ts
$ egrep END types-dnscontrol.d.ts  |grep -v D_EXTEND
 * `END` permits the last item to include a comma.
 * END)
declare const END: DomainModifier & RecordModifier;

That said, we can update the comment:

diff --git a/commands/types/others.d.ts b/commands/types/others.d.ts
index 185ede34..ee134c77 100644
--- a/commands/types/others.d.ts
+++ b/commands/types/others.d.ts
@@ -43,7 +43,9 @@ declare const CF_UNIVERSALSSL_ON: DomainModifier;
 declare function CLI_DEFAULTS(vars: Record<string, unknown>): void;
 
 /**
- * `END` permits the last item to include a comma.
+ * `END` permits the last item to include a comma.  As of v4.15.0 this
+ * is no longer needed as the JavaScript interpreter now accepts a
+ * trailing comma.
  *
  * ```js
  * D("foo.com", ...

@tlimoncelli
Copy link
Contributor

Sorry to rush you but I'm doing a point release today. I'm merging this. If further improvements are needed, at least doc-related PRs hit the website without waiting for a release.

@tlimoncelli tlimoncelli merged commit ee49704 into StackExchange:main Dec 17, 2024
6 checks passed
@cafferata
Copy link
Collaborator Author

Ready for merge?

Yes, it's was ready for merge. 👍🏻

@cafferata cafferata deleted the docs/code-tricks-end branch December 19, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs shouldn't refer to END
3 participants