Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Fix broken links #120

Merged
merged 7 commits into from
Jan 19, 2022
Merged

Fix broken links #120

merged 7 commits into from
Jan 19, 2022

Conversation

norswap
Copy link
Contributor

@norswap norswap commented Jan 10, 2022

Closes #115 and #116

Built on top on #117, don't merge!

Uses https://github.com/lycheeverse/lychee for link checking.

A bit unsure about the github workflow, needs testing. It works.

I'm expecting the workflow to fail at first, because we have one link to a build output, so either
we must add contract building to the workflow, or add an exception.

It passes, but that itself is a problem, because it's caused by lychee not checking absolute (/-prefixed) links.

@norswap norswap force-pushed the ns/fix-links branch 3 times, most recently from 79462b9 to 235ca32 Compare January 10, 2022 16:48
The bytecode to add to the genesis file will be located in the
`deployedBytecode` of the [JSON artifact file][l1-block-artifacts] corresponding
to L1Block.sol (this link will be broken unless you've built the contracts with
`yarn build` in the `packages/contracts` directory).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe just remove this link and display the path textually instead. @maurelian wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree. I made a suggestion below that I think gets this across well.

@norswap
Copy link
Contributor Author

norswap commented Jan 10, 2022

I couldn't get it to check absolute links. Opened an issue @ lychee here: https://github.com/lycheeverse/lychee-action/issues/70

In the meantime, I've changed the absolute links from deposits.md to be relative, except for the artifact links, which will therefore be conveniently ignored (though I think we should probably change that by a textual path, see my comment).

specs/deposits.md Outdated Show resolved Hide resolved
specs/glossary.md Outdated Show resolved Hide resolved
specs/glossary.md Outdated Show resolved Hide resolved
@norswap
Copy link
Contributor Author

norswap commented Jan 11, 2022

Following @trianglesphere's comment, I rechecked and found quite a few more broken link references (e.g. [text][link-ref]). Would be nice if the tool could catch those automatically too.

Lychee issue: lycheeverse/lychee#456

Copy link
Collaborator

@maurelian maurelian left a comment

Choose a reason for hiding this comment

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

I think it's good with these small changes

specs/deposits.md Outdated Show resolved Hide resolved
specs/deposits.md Outdated Show resolved Hide resolved
The bytecode to add to the genesis file will be located in the
`deployedBytecode` of the [JSON artifact file][l1-block-artifacts] corresponding
to L1Block.sol (this link will be broken unless you've built the contracts with
`yarn build` in the `packages/contracts` directory).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree. I made a suggestion below that I think gets this across well.

@norswap norswap force-pushed the ns/fix-links branch 2 times, most recently from 0d36e7b to fc555be Compare January 19, 2022 16:36
@norswap
Copy link
Contributor Author

norswap commented Jan 19, 2022

  • had to exclude twitter.com because of rate limiting
  • had to exclude email address because this
  • I'm explicitly listing files to lint more restrictively, otherwise this gets quite slow locally as it's trying to file markdown files in .git and node_modules
    • this does mean that I'm not currently linting markdown file in the javascript packages
    • we could add markdown files one by one though
    • this would fix it

@norswap norswap merged commit a3ffa9a into main Jan 19, 2022
@norswap norswap deleted the ns/fix-links branch January 20, 2022 17:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Links
3 participants