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

fix: broken internal links at multiple pages issue #1071 #1115

Merged
10 commits merged into from
Jun 25, 2024
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jun 3, 2024

Fixed a few broken links (not all) here.

  1. Link to https://hub.cosmos.network/delegators/delegator-guide-cli.html is fixed on two pages.
  2. Removed parts of text with outdated links in chain-integration docs. @dtribble Let me know if you want me to keep these parts and fix them with by linking something.
  3. Broken link fixed in coreeval/proposal.md
  4. Removed probably a typo link in zoe.md. @gibson042 Please confirm that it was a typo.
  5. Text display-software in ertp-data-types.md was linked to ui-component package which has been removed from repo - linked it to ui-kit repo now.
  6. Link to page "Cosmos Code with Us workshop" is broken as page was removed from cosmos website. I removed the link but kept the text.

This should fix all the internal broken links except the ones in chain-integration.md mentioned in #1071.

Copy link

cloudflare-workers-and-pages bot commented Jun 3, 2024

Deploying documentation with  Cloudflare Pages  Cloudflare Pages

Latest commit: a2109f4
Status: ✅  Deploy successful!
Preview URL: https://cd1afbfb.documentation-7tp.pages.dev
Branch Preview URL: https://ms-fix-broken-links.documentation-7tp.pages.dev

View logs

Copy link

github-actions bot commented Jun 3, 2024

Cloudflare deployment logs are available here

@ghost ghost marked this pull request as ready for review June 3, 2024 15:33
@ghost ghost requested review from dtribble, dckc, gibson042 and toliaqat June 3, 2024 15:35


| ![Alt name of image](./assets/cosmos-api.png) |
Copy link
Member

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?

Copy link
Author

@ghost ghost Jun 3, 2024

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.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@dckc dckc left a 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

Comment on lines 29 to 35
## 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
```

Copy link
Member

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 ?

Copy link
Author

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?

Copy link
Member

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.

Comment on lines 10 to 12
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.
Copy link
Member

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

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Comment on lines -367 to -368
<a href="offerargs"></a>

Copy link
Member

Choose a reason for hiding this comment

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

why remove this?

Copy link
Author

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?

Copy link
Member

@gibson042 gibson042 Jun 6, 2024

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.

Suggested change
<a href="offerargs"></a>
<a id="offerargs"></a>

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@ghost ghost changed the title fix: borken links to cosmos-delegators-cli issue #1071 fix: broken internal links at multiple pages issue #1071 Jun 5, 2024
@ghost
Copy link
Author

ghost commented Jun 20, 2024

@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.

@ghost ghost requested a review from dckc June 20, 2024 04:45
@ghost ghost merged commit ec94877 into main Jun 25, 2024
5 checks passed
@ghost ghost deleted the ms-fix-broken-links branch June 25, 2024 04:09
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants