-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
docs: add @shikiji/vitepress-twoslash
#5483
docs: add @shikiji/vitepress-twoslash
#5483
Conversation
✅ Deploy Preview for fastidious-cascaron-4ded94 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Do these two lines need changes as well?
Lines 3 to 4 in d400388
command = "pnpm docs:build" | |
ignore = "git diff --quiet $COMMIT_REF $CACHED_COMMIT_REF -- docs/ package.json pnpm-lock.yaml" |
- Before
pnpm docs:build
we needpnpm build
so that types are available on file system, right? - The
ignore
part may now skip builds when types are changed
There's also plenty of errors on Netlify's logs. Likely related to these issues.
Yes, I tweaked the command a bit.
I'm not sure and don't know exactly how to change this, can you give me some tips? |
I guess right now it's checking whether there are changes in Maybe it's fine to just remove the |
OK, I removed it. |
I don't think it should be removed. For new releases, this line doesn't matter because the version in package.json is always updated. For new PRs it's generally meaningless to build new documentation unless there is a change inside the docs themselves - otherwise, it will just create a lot of noise. |
With the current What pattern should we use in there? |
There is no way to rebuild documentation even now because it depends on the release cycle. If we need to rebuild it, we can always do so manually. Building documentation for every single PR sounds ridiculous to me. We added this line for a reason, and I'm not going back to times when we didn't have 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.
Thanks!
Can you resolve conflicts, please? 👀 |
877a330
to
2b40aa4
Compare
It has been solved, but it seems to cause some ci to fail. |
Thank you ❤️ |
Description
refer to vitejs/vite#16168
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.