-
Notifications
You must be signed in to change notification settings - Fork 14
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(javascript): clean rollup config #700
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
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.
Looks good ! I don't understand everything about rollup but it takes roughly the same time as before so nothing bad
import fs from 'fs'; | ||
|
||
// Org where the packages are pushed | ||
const NPM_ORG = '@experimental-api-clients-automation/'; |
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.
maybe that value could be read from a package.json
, should not be complicated if you want to do it
It's mostly splitting code in this one, there should not be perf changes but I'm doing some test to improve it and it was easier to navigate like that |
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.
Nice !
algolia/api-clients-automation#700 Co-authored-by: Clément Vannicatte <vannicattec@gmail.com>
🧭 What and Why
🎟 JIRA Ticket: -
Changes included:
I was experimenting some things to speed up the build process of the JS client and thought it was hard to navigate in the real part of the rollup config.
This PR split the concerns of the real rollup config and the package-related steps, there is no changes in the build itself.
🧪 Test
CI :D