Skip to content
This repository has been archived by the owner on Jun 28, 2019. It is now read-only.

Feature/typescript conversion #56

Merged
merged 3 commits into from
Oct 7, 2015

Conversation

ryan-nauman
Copy link
Contributor

I'm proposing that this project is converted to use TypeScript (autocomplete, error/warnings, etc). It picked up two errors that I've corrected.

If you aren't amenable to the switch I was hoping that the generated javascript would closely resemble yours so that I could continue using it on my fork. However, unfortunately it seems that the generated JS is hardcoded to use 4 space indentation.

I'm happy that you created this project. It's useful for me daily and I enjoy working on it. I hope you'll consider these changes. I don't want to permanently fork!

PS since #55 is open still it is bundled here as well.

@matt-h
Copy link
Owner

matt-h commented Oct 7, 2015

Hi Ryan,

Thanks for your interest in contributing. I'm not sure if moving to TypeScript is the best option for the project. For one, I don't use it myself so it will be one more thing to maitain for this project. It will also add another barrier for entry for someone to have to setup before being able to contribute.

It might be a good idea for us to setup Travis-CI with something like jslint for detecting JavaScript issues. Another option would be to switch to ES6/ES2015 using babeljs as that does seem to be where the JavaScript world is heading in the future.

I am not opposed to switching to TypeScript, I'll just have to think on it more if it will be good for the project in the long run. Do you plan to be implementing many more features? If so, do you know which ones you plan to tackle? If you want to be a large contributor and TypeScript is your preferred method of development to add those features then that would be a big factor in the Pro column for reasons to switch to TypeScript.

That all said, I have implemented your fixes that you added in this Pull Request here: d7847a1

@ryan-nauman
Copy link
Contributor Author

5 second typescript pitch incoming. I hate coffee script. With that said, TypeScript only has the learning curve you want it to. This pull req is the same vanilla JS you're used to except with some typing hints to resolve warnings.

I will implement the ones that interest me. #21 #51 #54 and maybe #11 if it's what I think it is. I'm working on adding yourself to the listener badges.

@matt-h
Copy link
Owner

matt-h commented Oct 7, 2015

I agreen on the coffee script hate and I see the changes were minor in the conversion. The big hurdle is just having to install the TypeScript compilier (but that is easy to do).

We can switch to TypeScript for now and see how it goes. It will be easy to switch back if it becomes an issue.

matt-h added a commit that referenced this pull request Oct 7, 2015
@matt-h matt-h merged commit ffe50bb into matt-h:master Oct 7, 2015
@ryan-nauman ryan-nauman deleted the feature/typescript-conversion branch October 7, 2015 20:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants