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.

Sign up for GitHub

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

Jump to bottom

Fix yarn check-readme #6300

Open
nflaig opened this issue Jan 15, 2024 · 5 comments · May be fixed by #6868
Open

Fix yarn check-readme #6300

nflaig opened this issue Jan 15, 2024 · 5 comments · May be fixed by #6868
Labels
meta-bug Issues that identify a bug and require a fix. scope-testing Issues for adding test coverage, fixing existing tests or testing strategies.

Comments

@nflaig
Copy link
Member

nflaig commented Jan 15, 2024

Maybe we should run yarn check-readme in CI along with tests to keep these readmes compile-able?

We actually do

lodestar/.github/workflows/test.yml

Lines 144 to 145 in 88744d8

- name: README check
run: yarn check-readme

But it doesn't seem to care at all

~/projects/ethereum/lodestar/packages/light-client [unstable ≡ +0 ~1 -0 !]> yarn check-readme
yarn run v1.22.19
$ typescript-docs-verifier
ℹ Compiling documentation TypeScript code snippets from README.md
ℹ Found 1 TypeScript snippets
✔ All snippets compiled OK

@jeluard how did you verify and determine the changes to make it compile, just copy pasted it into a .ts file?

Originally posted by @nflaig in #6297 (comment)
@nflaig nflaig added scope-testing Issues for adding test coverage, fixing existing tests or testing strategies. meta-bug Issues that identify a bug and require a fix. labels Jan 15, 2024
@jeluard
Copy link
Contributor

jeluard commented Jan 15, 2024

@nflaig yes just a plain copy

@jeluard
Copy link
Contributor

jeluard commented Jan 15, 2024 •
edited
Loading

Some findings:

  • it actually detects some things, like unknown variables at top-level, but not inside a function
  • if I comment "transpileOnly": true, all errors seem to be picked up

@matthewkeil
Copy link
Member

Nice catch!!! Your thoroughness is commendable @jeluard!!! It looks like we can pass a “—project” to the typescript-docs-verifier. Perhaps we have a tsconfig.docs.json with that set to false to make sure it runs correctly

This was referenced Jun 3, 2024
Merged
Draft
@philknows
Copy link
Member

@nflaig Any updates on your PR #6868 ?

@nflaig
Copy link
Member Author

nflaig commented Oct 22, 2024

@nflaig Any updates on your PR #6868 ?

hmm there was no response on bbc/typescript-docs-verifier#31 and the module we use seems unmaintained, maybe we just fork it? I haven't found an alternative

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
meta-bug Issues that identify a bug and require a fix. scope-testing Issues for adding test coverage, fixing existing tests or testing strategies.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: fix verification of README code snippets ChainSafe/lodestar
4 participants
@jeluard @matthewkeil @nflaig @philknows