-
-
Notifications
You must be signed in to change notification settings - Fork 40
Avoid CLI re building the project if it's already built #187
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
Conversation
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 work @gboutte!
Thank you for migrating the playground to TypeScript together with solving #186.
The code change looks good, it's just that the test you've wrote seems to be breaking the e2e
(Cypress) tests which run after the CLI tests. I've debugged it a bit and it seems the fact that you (re)move files in the dist folder causes the dist to be corrupted after the tests have ran.
Could you take a look at that? Maby you could find out a different way to run these tests. Or if that doesn't work, maby you can try to build the playground again, after running the integration tests, restoring the dist folder.
I looked at this problem.
I did some tests by removing my changes and my tests, and if I run I think that the I don't know if it's possible to make both result the same. I made a fix on the tests so that the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #187 +/- ##
==========================================
Coverage ? 100.00%
==========================================
Files ? 2
Lines ? 22
Branches ? 3
==========================================
Hits ? 22
Misses ? 0
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
It's interesting that strapi build
and compileStrapi
don't give the same output in the dist folder.
Anyways, this solution is fine for the scope of this issue. Thanks again! I'll get this released asap.
What does it do?
Why is it needed?
In some cases the strapi project is deployed in a readonly mode, and the project is already built. So if we run the CLI in the environment we will get permission error to write.
How to test it?
In a typescript project, the first CLI run will build the project (if not alrady built). To verify that it's not re building we can manualy change something in the dist folder, and re run the CLI. Since the dist folder already exist, it won't rebuild.
Related issue(s)/PR(s)
Fix issue #186