Skip to content

Js.Promise -> Js.Promise2 #3313

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 23, 2019
Merged

Js.Promise -> Js.Promise2 #3313

merged 1 commit into from
Jan 23, 2019

Conversation

chenglou
Copy link
Member

No description provided.

@chenglou
Copy link
Member Author

Depends on #3312

@chenglou chenglou force-pushed the storage2 branch 2 times, most recently from fd74d35 to 58068a5 Compare January 23, 2019 02:56
bobzhang
bobzhang reviewed Jan 23, 2019
View reviewed changes
jscomp/test/ocaml_parsetree_test.ml Outdated
|> Parse.implementation
|} with
|} with
| {Parsetree.pstr_desc =
Parsetree.Pstr_value (Asttypes.Nonrecursive,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, trust me, having your editor to trim whitespace will cause your more trouble than it's worth..
there are quite a few cases where source code is not whitespace insensitive -- e,g. our encoding of .bsbuild is whitespace sensitive

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see.

The problem is having trimming is the baseline for most contributors. We should adapt for the contributors, not make them adapt to us for one or two bugs potential here (which would have been caused anyway if someone accidentally adds a space)

@chenglou
Copy link
Member Author

I've regenerated jscomp/test/ocaml_parsetree_test.ml after changes to mt.ml. Now there are some hard-coded paths that changed, like:

let standard_library_default = "/Users/hongbozhang/git/bucklescript/vendor/ocaml/lib/ocaml"

into

let standard_library_default = "/Users/chenglou/Github/bucklescript/vendor/ocaml/lib/ocaml"

Otherwise the tests are passing again. I assume this is fine.

@chenglou chenglou mentioned this pull request Jan 23, 2019
Merged
@chenglou chenglou merged commit daa97f1 into rescript-lang:master Jan 23, 2019
@chenglou chenglou deleted the storage2 branch January 23, 2019 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobzhang bobzhang bobzhang left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chenglou @bobzhang