-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
fix(docs): simplify examples #968
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks, this looks a lot better! :-) Some comments inline.
You do not need to close and re-create the pull request when you make changes. You can force-push to your branch, and the PR will update automatically.
docs/dyndns/update-api.rst
Outdated
@@ -172,12 +172,9 @@ affiliated with the respective account (see :ref:`manage-tokens` for details.) | |||
stand-in for an IPv6 address. Replace those (including the ``<`` and ``>``) | |||
with your respective values. | |||
|
|||
**Examples with headers** (recommended) |
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.
Please use some kind of subheading (not a regular paragraph)
docs/dyndns/update-api.rst
Outdated
|
||
curl "https://update.dedyn.io/?hostname=<your domain>&myipv4=1.2.3.4&myipv6=fd08::1234" \ | ||
--header "Authorization: Token <your token secret>" | ||
|
||
**Examples without headers** (not recommended) |
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.
Please use some kind of subheading (not a regular paragraph)
docs/dyndns/update-api.rst
Outdated
|
||
Basic authentication with automatic IP detection (IPv4 **or** IPv6):: | ||
|
||
curl --user <your domain>:<your token secret> https://update.dedyn.io/ |
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.
Please indent correctly like in examples above. Also applies to examples below.
docs/dyndns/update-api.rst
Outdated
|
||
Basic authentication with forced use of IPv4 (while preserving the IPv6 address from the DNS):: | ||
|
||
curl --ipv4 --user <your domain>:<your token secret> "https://update.dedyn.io/&myipv6=preserve" |
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.
Quoting from #956 (review):
- Please make sure that all examples work
- Please make sure that all examples are technically correct
Have you tested this?
The first query parameter should be indicated with ?
, not with &
. (The quotes can then also go away, they are also needed because Linux command lines have special treatment for &
.)
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.
Please see inline for nits and rendering issues.
When you're done, please squash and re-request review. Thanks :-)
No description provided.