search

Bridgewater / scala-notebook

Interactive Scala REPL in a browser
Other
741 stars 155 forks source link

Fix for Issue 12, custom kernel classpath #13

Closed chrismyang closed 11 years ago

chrismyang commented 11 years ago

did a tiny refactor to that method, just to make it slightly clearer what's going on.

copumpkin commented 11 years ago

That downgrade commit (f423812) seems a little weird, when we don't have Play (yet?) and I don't see it in your repo either. I'd also expect them to make a release compatible with more recent versions in the near future, if we end up needing it. The other commits seem good to me.

chrismyang commented 11 years ago

Whoa, I didn't intend to add the other commits to the pull request -- just 0034028. I am a GitHub noob -- what should I have done instead? Created a new branch and submitted that branch?

copumpkin commented 11 years ago

Ah, the problem is that your 0034028 "depends on" the earlier states of the repo according to the git metadata, even though semantically it doesn't (you need darcs!)

I haven't used cherry-pick much, but I think http://stackoverflow.com/questions/5256021/send-a-pull-request-on-github-for-only-latest-commit covers what you want here. In general, I think the conventional workflow is to put distinct "features" (however you define that term) into different branches and merge the stuff you're ready for people to see into master. In that model, your Play work might have been in a distinct branch and your bugfix would have gone straight into master. You might have then merged your bugfix into your Play branch, and submitted a pull request from master for the rest of the world to see.

Of course, when Ken merges this, he can also just avoid merging parts of it he doesn't want, but precise pull requests are probably best.

Bridgewater commented 11 years ago

As suggested, I just merged 0034028, the change to the classpath.