search

purescript-deprecated / gulp-purescript

Gulp plugin providing PureScript compiler tasks
34 stars 8 forks source link

Adding support for PureScript 0.9.1 and newer #61

Closed ethul closed 8 years ago

ethul commented 8 years ago

Resolves #60 and resolves #59

@garyb Just wondered if you had any thoughts on this PR. Thanks!

kritzcreek commented 8 years ago

Skimmed through and it looks good! Only two things related to imports. Why did you explicitly write out Data.Array members when you are fully qualifiying it anyway? And you don't need to explicitly list Prelude members since you can have one implicit import per file.

ethul commented 8 years ago

@kRITZCREEK Thanks for taking a look!

I see your point about Data.Array. I suppose there is no benefit to write out the members being used since it is qualified. I can update this.

For the Prelude, I am thinking that it is clearer to see what is being used. Is there a drawback to writing out the members here? Or is it is not considered proper practice?

kritzcreek commented 8 years ago

No drawbacks. Just something I regularly see and do... but I actually kind of like the explicit imports here because they're few

ethul commented 8 years ago

Sounds good. I've updated Data.Array. I will push the release tonight or tomorrow. Thanks again!

garyb commented 8 years ago

Looks great to me :+1:

I think Aff has been updated now too, so you should be able to use a real dependency for that.

ethul commented 8 years ago

Great! Thanks for taking a look. I will update the Aff dependency.

On Sunday, 5 June 2016, Gary Burgess notifications@github.com wrote:

Looks great to me 👍

I think Aff has been updated now too, so you should be able to use a real dependency for that.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/purescript-contrib/gulp-purescript/pull/61#issuecomment-223847225, or mute the thread https://github.com/notifications/unsubscribe/AAVYy95H28UuSEmgyEpJ9ah7JbD-FW5Uks5qI2jogaJpZM4IuO8b .