-
Notifications
You must be signed in to change notification settings - Fork 2.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
chore(ci): run TypeDoc generation in Code Quality Checks action #4345
Conversation
Outdated file: `typedoc` is only a dependency of `website/`, and this file lacked the latest updates requried for recent typedoc compatibility.
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/invertase/react-native-firebase/ngg1uqv9e |
id: yarn-cache | ||
with: | ||
path: ${{ steps.yarn-cache-dir-path.outputs.dir }} | ||
key: ${{ runner.os }}-yarn-${{ hashFiles('**/package.json') }}-with-website |
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.
interesting I think if you just put the yarn-cache from checkout package.json doesn't have the full thing yet so that key won't be right. This seems like a fine workaround given that it's not in the workspace so not in package.json by default.
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 believe the decisions you made on the items you highlighted are sane, from my perspective, I'm +1 but I'll mark for second review just in case
Thanks for digging in to this as well!
@Salakar the tradeoffs enumerated above are worth a second look I think - look good to me, but just in case... |
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.
This looks great to me and the PR was well written, so tyvm for that!
Related issues
Fixes #4321
Description
This PR adds TypeDoc generation in the Code Quality Checks action. In PR #4306, a typedoc failure was causing the Vercel deployment phase to fail, and those logs aren't public.
There were a few challenges that required decisions:
typedoc
is a dependency ofwebsite/
, which is not part of the lerna/workspace, so its dependencies are not installed in current actions.Yarn Install (Website)
stage, with the existing retry pattern.-with-website
) since additional packages are installed. Not certain this is correct, so be sure to reviewtypedoc
as a root dependency, or do a one-offyarn add typedoc
in the CI actionscripts/generate-typedoc.js
script that is no longer used. It did not receive the recent typedoc changes that thewebsite/scripts/generate-typedoc.js
file did, so it is not currently functional.website/scripts/generate-typedoc.js
file is not a callable script.node -e "require('./website/scripts/generate-typedoc').generateTypedoc()"
scripts/generate-typedoc.js
file as a callable script, or alter thewebsite/scripts/generate-typedoc.js
to be more flexibleThanks for reviewing!
Release Summary
N/A
Checklist
Android
(N/A)iOS
(N/A)e2e
tests added or updated inpackages/\*\*/e2e
(N/A)jest
tests added or updated inpackages/\*\*/__tests__
(N/A)Test Plan
CI actions
https://github.com/invertase/react-native-firebase/pull/4345/checks?check_run_id=1196025300
Think
react-native-firebase
is great? Please consider supporting the project with any of the below:React Native Firebase
andInvertase
on Twitter