-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add js.event, js.json & js.yaml, using JS yaml
package
#20
Conversation
Thanks, this looks good at a first glance. |
yaml
packageyaml
package
I added a test harness to also generate an event stream. I also configured my local instance of yaml-test-matrix to use this; the only tests that are reading as errors for me are Anything I can do to help get this merged? |
@perlpunk @ingydotnet Anything I could do to help move this ahead? |
We're both pretty busy, preparing for another conference in two weeks... |
At the conference I tried out the PR, but got an error:
|
I tried copying the source directory to /tmp to avoid changing the original, but getting a different error now.
I pushed my changes to branch @ingydotnet would be nice if you could have a look |
@perlpunk Could it be that the permissions for your |
@eemeli I think it's because the build process changes the source directory (adds directories and files). I actually started to change all the build scripts to first copy the source directory to /tmp, because in another case it resulted in a newer change time for the directory which made |
Ah, now I see. I'm pretty sure that the problem is due to the updated npm handling I just pushed a fix that should work around this issue. |
Good news! I was able to build. I'll run the test-matrix to check if everything is still working (since one of the underlying images were updated, so probably some ubuntu package updates). I'm still not quite happy with the naming. Maybe we could prefix the js packages with |
For some reason ruamel.yaml is now broken. It adds |
ok, the For some reason our setup isn't installing our cloned hg source of ruamel, but the latest pypi version. Will have to fix that later. I pulled the ubuntu image and now rebuilding, and hopefully I'll get the fixed version of ruamel. |
Ok, rebuild looks good, just running the test matrix on your processor, excellent results! Will merge as soon as possible, maybe not tonight though |
sorry, this was a crazy week. Thanks, your project looks very promising and currently has the best results, and I hope I get time to look a bit closer at it soon! |
Forgot to close ;-) |
This adds
yaml
to the editor. Its JS package is currently installable withnpm install yaml
, but I've here followed the pattern of other packages and built it from source. It should pass almost all of the applicable test-suite tests, but it's not instrumented (at least yet) for the event format.As
yaml
requires at least node v6, I've also updated the config to use node v8, the current LTS version that first came out in 2017.There's a slight naming conflict that this package introduces, as the other JS package is
js-yaml
, and I couldn't figure out a better naming pattern than calling mine justjs
. If you can recommend an alternative, I'd be happy to rename the rules for one or both libraries.