Glance-Bookmarklet
Glance-Bookmarklet copied to clipboard
Miserlou
Added Ability to select new text and pause. Also added a bookmarklet.html page.
I have made code changes to allow OpenSpritz to Pause the current Spritz. Also you can now select new txt and continue reading a new selection.
For a pull request the annoying thing is that I have references to my github account that need to be changed back to yours.
Additional comment I was trying to get keybindings to work with the jQuery hotkeys plugin. I didn't have time to figure out what was wrong. Hope these feature make it into your plugin.
The pause feature is crucial. Excited to see this get added to the codebase.
@smorin, thanks for doing all this work, it looks awesome and I'm glad you are so interested in this project. My guess though is that this pull request won't be merged by @Miserlou
A couple of housekeeping issues holding it back: please make sure pull requests are against the dev branch. Make sure you squash your commits so that it doesn't add a ton of commits to the main repo. Most importantly, pull requests should be as small in size as possible and only changing one thing in each. In this case, you are adding pause (which another pull request added), continuing on a new selection, key bindings, and added a ton of logic in spritzify_go(). Also, this is using the old jQuery version, which the dev branch (and everything moving forward) doesn't use.
The reason this should be many different pull requests is so that each feature can be reviewed separately. For example, just because another pull request added pause, this won't ever be merged. However if your pause was a separate pull request from everything else, we could ignore it and look at the other ones.
While it might seem like I am trying to shoot down your work, I am actually really truly glad you care enough about this project to do as much as you did. I want to see you be successful and get your changes merged which is why I took the time to write out this comment.
No problem.
All your comments make sense.
The truth of the matter I was in a rush have don't have time to fix it. The changes are there so should be easy for someone to take them and format them to roll the changes in.
I am currently running off my fork of the project.
On Tue, Mar 11, 2014 at 4:50 PM, Eli White [email protected] wrote:
Smorin, thanks for doing all this work, it looks awesome and I'm glad you are so interested in this project. My guess though is that this pull request won't be merged by @Miserlou https://github.com/Miserlou
A couple of housekeeping issues holding it back: please make sure pull requests are against the dev branch. Make sure you squash your commits so that it doesn't add a ton of commits to the main repo. Most importantly, pull requests should be as small in size as possible and only changing one thing in each. In this case, you are adding pause (which another pull request added), continuing on a new selection, key bindings, and added a ton of logic in spritzify_go(). Also, this is using the old jQuery version, which the dev branch (and everything moving forward) doesn't use.
The reason this should be many different pull requests is so that each feature can be reviewed separately. For example, just because another pull request added pause, this won't ever be merged. However if your pause was a separate pull request from everything else, we could ignore it and look at the other ones.
While it might seem like I am trying to shoot down your work, I am actually really truly glad you care enough about this project to do as much as you did. I want to see you be successful and get your changes merged which is why I took the time to write out this comment.
Reply to this email directly or view it on GitHubhttps://github.com/Miserlou/OpenSpritz/pull/63#issuecomment-37360612 .
Steve Morin | Hacker, Entrepreneur, Startup Advisor twitter.com/SteveMorin | stevemorin.com Live the dream start a startup. Make the world ... a better place.
It seems, the pause is included, but the selection of text is not, right? I would find this a very valuable extension!
