faker
faker copied to clipboard
infra(ci): run against playground
Main types index.d.ts doesnโt exist.
Fix types to match '.' exports.
ref: https://github.com/jhipster/generator-jhipster/pull/27179
Deploy Preview for fakerjs ready!
Built without sensitive environment variables
| Name | Link |
|---|---|
| Latest commit | 356e3ae35fdd041740ceee66dee36b070087e93b |
| Latest deploy log | https://app.netlify.com/sites/fakerjs/deploys/66e03332d2c0220008ede2e5 |
| Deploy Preview | https://deploy-preview-3095.fakerjs.dev |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 99.97%. Comparing base (
18ab2c7) to head (356e3ae). Report is 174 commits behind head on next.
Additional details and impacted files
@@ Coverage Diff @@
## next #3095 +/- ##
==========================================
+ Coverage 99.96% 99.97% +0.01%
==========================================
Files 2776 2776
Lines 226260 226260
Branches 945 591 -354
==========================================
+ Hits 226183 226207 +24
+ Misses 77 53 -24
Same fix is applied in https://github.com/faker-js/faker/pull/3094. Alternative fix is applied in https://github.com/faker-js/faker/pull/3093 (removes types from root)
The above PRs tries to fix CommonJS types too, while this fixes esm types.
you can checkout https://github.com/faker-js/playground and test your changes
you can checkout https://github.com/faker-js/playground and test your changes
Why donโt integrate playground on ci tests?
you can checkout faker-js/playground and test your changes
Why donโt integrate playground on ci tests?
PR welcome, go and try for it
@mshima thanks, this is a huge step forward and I wished for something like this a long time โค๏ธ you need to rebind/relink the repo faker to the playground test run so something like modifying pnpm override before installing the playground deps
@Shinigami92 I am on mobile now, so I cannot rebase. I think itโs only missing steps names now. Feel free to modify the PR.
Playground is passing ๐
Playground seems to be missing pure node ts samples.
AND NOW we really see the result! ๐
Right now, this behavior is correct! because we have this issue
For some reason mapping types to non existing index.d.ts fixes playground.
In the end it just adds the playground workflow
Please note that this will create a bi-directional dependency between two separate repositories.
Please note that this will create a bi-directional dependency between two separate repositories.
Do you see any problem with this? IMO it's fine, because playground is in out org as well.
IMO running playground in CI:
- reduces maintenance burden by avoiding regressions and requiring manual checks: https://github.com/faker-js/faker/pull/3093#issuecomment-2339729022.
- improves contribution experience by not having to ask to manually check against playground: https://github.com/faker-js/faker/pull/3095#issuecomment-2335533878
We could do that but it has a side effect, that all features that are used in the playground that might experience breaking changes need to be updated to the new feature before it is available as a dependency. This is also the case for the releasenPR but that those are even rarer.
- the workflow can be ignored if it's not required to pass.
- contributor will be warned about breaking changes or unwanted breaking changes.
Moving playground to the main repository can be considered too.
- the workflow can be ignored if it's not required to pass.
- contributor will be warned about breaking changes or unwanted breaking changes.
I agree ๐ we can mark the CI check as "not required" and if it breaks something, we are just "informed"
I agree ๐ we can mark the CI check as "not required" and if it breaks something, we are just "informed"
Please note that that will only work once and will cause confusion for future PRs (that will fail the CI even if they didn't break anything because next is already "broken").
I would rather move the playgrounds inside the main repo to avoid these bidirectional dependencies.
Team Decision
We don't want to add bi-directional dependencies between the playground and the faker repo. We think that moving the playground to the main repo is the best solution for that, but that would require extensive work e.g. converting the repo into a mono-repo, what we don't want to do yet.
We would like to still run the pipeline on demand somehow e.g. using a comment, but we are currently unsure about the exact expectation we have for the pipeline. We will talk about this again in a future team meeting.
If you have an idea, on how to move the playground to the main repo feel free to open a PR with a suggestion.
We are looking into restructuring our project setup to solve this issue. For now, you don't have to update this PR. Thanks for bringing this to our attention.