Update to lenfire's more recent derby and work around ref issue. #340

Merged
merged 3 commits into from Dec 5, 2012

Projects

None yet

5 participants

Contributor

Fixes todomvc derby issue:

#338

which is itself due to a derby issue:

derbyjs/derby#171

by updating to use lenfire's more recent version of derby. When I updated, there was an issue with one of the model.ref's which I had to work-around also.

I'd be keen for @lenfire to verify whether the commit ref I'm refering to in package.json will be permanent (I didn't want to refer to master, but am happy to change that if it suits lenfire's work-around).

Owner

Sounds good to me. If the review clears I'm happy for us to merge this. Appreciate the pull request.

Contributor

@lefnire - if you get a chance, can you confirm whether this is the right work-around based on derbyjs/derby#171 ? (in particular, refering to a particular commit on your branch - I'm assuming you won't be overwriting history).

lefnire commented Dec 4, 2012

@absoludity I'm not familiar with the particular issue, but if this fixes it - √
As far as my fork, it only exists to use racer's git HEAD. I'll try to keep it around and up-to-date - the only reason I might want to scrap it is to start over to create clean pull requests for other things, but if that happens I'll come back here to with fair warning.
Maybe we can convince @codeparty to use "racer": "git://github.com/codeparty/racer#master" for all commits between tags.

Contributor

Thanks @lefnire - the particular issue was

derbyjs/derby#171

but then after updating to use tip (via your fork) I hit another issue related to keyed refs that I only see on tip. Anyway, I've switched the dependency on your fork to use #master (as given that your fork just updates the racer dependency to #master, we don't win any stability by pinning to a commit), as well as documented the keyed-reference issue that I hit:

derbyjs/derby#179

in the code so we can switch back once it's resolved.

Thanks for the heads-up @sindresorhus and @addyosmani. Maybe we can get @lackac to take a quick look, or otherwise land this (make run with these changes results in the working app).

Contributor
lackac commented Dec 5, 2012

It looks good to me.

@sindresorhus sindresorhus merged commit d3d4381 into tastejs:gh-pages Dec 5, 2012
Owner

Cool. Thanks for the fix @absoludity and thanks for confirming @lackac

lefnire commented Dec 28, 2012

see the latest edge instructions on using codeparty/derby & codeparty/racer instead of lefnire/derby

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment