Skip to content
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

Upgrade to Nagareyama #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alfonsogarciacaro
Copy link
Contributor

First of all, I must say this is a great project! I feel embarrassed for not having checked it before. Could you please add it to the community.json in the Fable website so it's easier for others to find? https://github.com/fable-compiler/community/edit/master/public/community.json

Thanks to your wonderful tests, I could find (and fix) a bug that went under the radar of the tests in Fable repo. Now everything is green! I had to make a few changes to make it build in my system (macOS), I hope I'm not breaking anything for you:

  • Paket was insisting to use mono which I have not installed. Deleting the .paket/ folder seemed to fix it, I have also added it to .gitignore to remove it from the repo.
  • Also did a few changes in build.fsx.
  • Because Nagareyama doesn't use Babel anymore, tests are compiled to ES2015 modules. Instead of adding an additional step, I added a babel.config.js file that instructs Jest to convert the modules on the fly.
  • splitter.config files are not needed anymore, I'm just passing the arguments in the "pretest-*" scripts of package.json. Note that I'm using the --run option to load the SnapshotLoader after compiling Fable.Jester.Tests.

I hope you like how the new Fable works. Please give it a try when you have a moment and let me know if it works for you.

@Shmew
Copy link
Owner

Shmew commented Oct 2, 2020

First of all, I must say this is a great project!

Thanks!

Could you please add it to the community.json in the Fable website so it's easier for others to find? fable-compiler/community/edit/master/public/community.json

It's already present 😃.

All of the points you raised look good to me, not needing the somewhat complicated splitter config is a huge bonus in my book! I'm seeing the snapshot test fail in CI, but if it's passing locally I think it just needs to be updated.

@alfonsogarciacaro
Copy link
Contributor Author

Ah, seems community.json had an error and it wasn't loading properly so an old version was being used instead and I couldn't see your package. It's fixed now 👍

@github-actions
Copy link

github-actions bot commented Dec 2, 2020

Stale pull request message

@Shmew Shmew force-pushed the master branch 2 times, most recently from f403788 to 8c9830c Compare January 28, 2021 16:03
@github-actions
Copy link

Stale pull request message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants