Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guide to my messy PRs #38

Closed
EricForgy opened this issue Dec 29, 2015 · 7 comments
Closed

Guide to my messy PRs #38

EricForgy opened this issue Dec 29, 2015 · 7 comments

Comments

@EricForgy
Copy link
Contributor

Hi @one-more-minute and @Varanas,

I'm sorry you get to suffer through my learning how to contribute to packages, but Blink happens to be potentially very useful to me so hopefully you can take that as a positive :)

Anyway, I'm afraid I made a mess, so I'll try to organize my PRs here. I'll update this as I go.

As a start, I think #37 is the easiest and, if you agree, can be merged first.


Merged. Yay!
@EricForgy
Copy link
Contributor Author

After #37, you can have a look at #35. My first commit on #35 already had the node.js side of #37, but I just stripped that out so #35 only has the blocking code changes. I think making the API synchronous is consistent with the rest of Blink.


Oops. I closed #35 because I needed to make a revision, I made the revision, and force pushed it, but force-pushing a closed PR makes the PR permanently closed :sweat_smile:  

~~~**PR to look at:** #45~~~

Closed in favor of #43

@EricForgy
Copy link
Contributor Author

Initially in #36, I combined a "close" method (closing #28) with #35, but decided to split the two so I just submitted #39 that just adds a "close" method to the API. Messy. Sorry about that.


Merged. Yay!

@EricForgy
Copy link
Contributor Author

I finished what @GlenHertz started in #21


Merged. Yay!

@EricForgy
Copy link
Contributor Author

With PR #43, loading a URL blocks until the page is loaded. Uses the same idea as #35.

PR to look at: #43

@MikeInnes
Copy link
Contributor

Haha, no problem, having lots of PRs to deal with is one of the better problems I've had ;) Happy to answer any questions you have as well, it's really great to see people using / contributing to Blink.jl.

@EricForgy
Copy link
Contributor Author

Ok @one-more-minute , @Varanas and @spencerlyon2

All these PRs are now just down to #43 :)

@sglyon
Copy link
Contributor

sglyon commented Jan 8, 2016

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants