-
Notifications
You must be signed in to change notification settings - Fork 39
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: broken internal links at multiple pages issue #1071 #1115
Conversation
Deploying documentation with Cloudflare Pages
|
Cloudflare deployment logs are available here |
|
||
|
||
| ![Alt name of image](./assets/cosmos-api.png) | |
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.
that image was pretty handy - are you proposing to omit it?
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 honestly don't know whether to keep or remove it - this page does not exist anymore though.
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.
ugh. I suppose that's worth its own issue:
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.
@dckc reckon we can merge changes to the rest of the files?
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.
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.
a few more suggestions
## Building `agd` | ||
|
||
The `agd` command line tool can be built as described in the Agoric [getting-started documentation](https://docs.agoric.com/guides/getting-started#build-the-cosmic-swingset-package). The linked step builds `agd`. To confirm that `agd` is in your `$PATH`, execute | ||
``` | ||
agd version --long | ||
``` | ||
|
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.
Rather than deleting this section, how about adapting info from the Installing agd box in https://docs.agoric.com/guides/agoric-cli/agd-query-tx.html ?
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 info there does not say much about building agd though - it is about how to use an already built agd
in docker container. Should I change the heading of the subsection to Installing agd too?
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 has 2 options: a docker container or building from a release.
Maybe the thing to do is to keep this section as "Building agd" and make it mostly a pointer to https://github.com/Agoric/agoric-sdk/releases ; the docker option is probably less relevant here.
This is the OTC Desk contract from the ["Building a | ||
This is the OTC Desk contract from the "Building a | ||
Composable DeFi Contract" episode of Cosmos Code With | ||
Us](https://cosmos.network/series/code-with-us/building-a-composable-defi-contract). | ||
Us workshop. |
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.
rather than removing this link, I suggest pointing it to https://www.youtube.com/watch?v=e9dMkC2oFh8
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.
Done.
<a href="offerargs"></a> | ||
|
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.
why remove this?
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 can bring it back - does it serve any purpose?
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, preserving old anchors allows any external links that use it to continue working (in this case, 45035c2 removed the "OfferArgs" heading but we still want its [formerly auto-generated] "offerargs" anchor to be associated with the replacement "Offer Arguments" heading). But you're right that there's a typo.
<a href="offerargs"></a> | |
<a id="offerargs"></a> | |
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.
Fixed.
@dckc I have removed changes to the chain-integration.md (further deliberation required) and fixed the rest. Let me know if this is good to go now. |
Fixed a few broken links (not all) here.
Removed parts of text with outdated links inchain-integration
docs. @dtribble Let me know if you want me to keep these parts and fix them with by linking something.display-software
in ertp-data-types.md was linked toui-component
package which has been removed from repo - linked it toui-kit
repo now.This should fix all the internal broken links except the ones in
chain-integration.md
mentioned in #1071.