search

meteor / todos

The example app "Todos", written following the Meteor Guide
Other
535 stars 367 forks source link

Upgrade CoffeeScript branch to Meteor 1.4.2.3 #206

Closed abernix closed 7 years ago

abernix commented 7 years ago

While investigating meteor/meteor#7459, I found it helpful to update the Todos app to 1.4.2.3 from 1.3.4.4.

Tests pass and it seems to all work!

This allowed the reverting of dbdaa9d56405dc744d56a016c56467bce26c6d66 which was in place previously to solve a circular dependency issue and is no longer a problem when using ES modules (now supported natively in CoffeeScript).

/cc @GeoffreyBooth as I believe you're usually responsible for the CoffeeScript side of this! Your input would be appreciated! :smile:

GeoffreyBooth commented 7 years ago

The tests are testing the .js files, not the .coffee files. Those references need to be updated.

Also this branch needs to be updated with the newer commits on master.

abernix commented 7 years ago

@GeoffreyBooth I did test it all manually too and it worked fine.

I figured it would need some features ported over, but it would seem that updating tests and adding new commits from master (and the consuming part: porting that code to CoffeeScript) are both unrelated tasks so maybe it would make sense to get this merged in order to be presenting the most recent version of Meteor sooner rather than later?

Unless you feel it would be easier to do those fixes and then rebase this on top of those?

GeoffreyBooth commented 7 years ago

Thanks @abernix. I merged this in and updated the code based on all other changes between a3bc010e7ecddb1ff7aa8f77ea2b14634d85e9ae (the last commit on master that I ported from) and d1edea007b828dec7316d40438350c0b76c92645 (the current commit as of now). It looks so much cleaner!

Still need to get the tests to work with the coffee files, but I figured I might as well release this for now. Even if not all the tests pass or there are small bugs, it’s at least a good reference for reading the code.

abernix commented 7 years ago

Awesome. Thank you, @GeoffreyBooth! Agree on both points. I opened #209 just so we don't lose track of the tests situation.