search

Schniz / bs-faker

Faker.js bindings for BuckleScript in Reason
MIT License
17 stars 8 forks source link

Split modules to separate files #36

Closed miluoshi closed 5 years ago

miluoshi commented 5 years ago

I was not sure about "update name to faker in bsconfig.json" in the issue description because of [1] so I kept it unchanged. Feel free to update it, I guess you know better.

[1] https://bucklescript.github.io/docs/en/build-configuration.html

Note: the bsconfig.json name should be the same as the package.json name, to avoid confusing corner-cases.

closes #33

Schniz commented 5 years ago

Woah, yeah. Appears like it should be BsFaker.Internet now - because this is the name in package.json. Can you change the README too? maybe merge your other contributions? then we'll release 1.0 so it would be a breaking change

Thanks!

Schniz commented 5 years ago

Great work, by the way. You're on fire 🔥

miluoshi commented 5 years ago

haha thanks! 🔥 Sure I'll update the readme file.

miluoshi commented 5 years ago

Merged Internet and Hacker from opened PRs.

miluoshi commented 5 years ago

Also, I am not sure about bindings coverage count now, someone should count the correct number 😄

Schniz commented 5 years ago

well, my friend, this is just amazing. thank you so much. Happy Hacktoberfest! 🎉

miluoshi commented 5 years ago

Glad to help! Thank you for the cooperation too! To be honest, this is my first major open-source contribution. I chose this repo because I am learning OCaml/ReasonML, so I wanted to practice it and this was the only OCaml repo on GitHub I found with issues tagged as Hacktoberfest 😄 Happy Hacktoberfest! 🎉

miluoshi commented 5 years ago

Oh, I just found one more typo. In the README.md the title ### BsFaker.fake should be now ### BsFaker.Faker.fake. Although it looks strange to me like this. Is there a way to have the fake function defined in the "root" BsFaker module, so it stays BsFaker.fake?

Schniz commented 5 years ago

I guess no 🤷‍♂️ We could have gone with module names like Faker_internet, and then have BsFaker.re:

module Name = Faker_name;
/* all the other files */
Schniz commented 5 years ago

that would be without namespace, but I guess it is fine too... maybe BsFaker.Generate.fake()?