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

fixed parsing of swarmUrl in inject.js #228

Closed
wants to merge 3 commits into from
Closed

fixed parsing of swarmUrl in inject.js #228

wants to merge 3 commits into from

Conversation

s-a
Copy link
Contributor

@s-a s-a commented Sep 16, 2012

Hello,
I added latest google chrome (21) and fixed parsing of swarmUrl in inject.js if URL with hash tag was given.

@s-a
Copy link
Contributor Author

s-a commented Sep 18, 2012

Regarding to https://github.com/s-a/testswarm/commit/d4191c3f0fcfc62e53aeaee1f0b1e5a6ce677870 there are many files added by mocha sources. It is for sure more usefull with a job sample loading assets from github.

However,
the main changes are here https://github.com/s-a/testswarm/blob/d4191c3f0fcfc62e53aeaee1f0b1e5a6ce677870/js/inject.js#L444

@Krinkle
Copy link
Member

Krinkle commented Sep 19, 2012

Create a new branch in your fork for this bug fix instead of doing it from the same pull request as for master. It doesn't work this way because other commits to your master now get into this merge request.

Like this:

$ git remote add upstream git://github.com/jquery/testswarm.git
$ git branch fix-inject-swarmUrl -t upstream/master
$ git checkout fix-inject-swarmUrl
# .. edit files ...
$ git add js/inject.js
$ git commit
# Push the new branch  (now in HEAD) to your GitHub account (aka "origin") 
$ git push origin HEAD

And then online at https://github.com/s-a/testswarm/tree/fix-inject-swarmUrl you can create a pull request.

Thanks!

@s-a
Copy link
Contributor Author

s-a commented Sep 19, 2012

Sorry, this is all new for me.
I will try it this evening! Thanks for your detailed description.
So if I understand correct I have to create branches for every pull request from upstream/master?

Or is it ok to create the next branch from "fix-inject-swarmUrl" for a second pull request?

Best regards

@Krinkle
Copy link
Member

Krinkle commented Sep 19, 2012

If you want to update the existing pull request (in other words, fix it) then commit to the same branch again, the pull request will automatically update itself (it is linked to the branch name).

When you want to make a new unrelated change, use a new branch and make sure it is fresh (-t upstream/master).

@Krinkle
Copy link
Member

Krinkle commented Sep 19, 2012

I'm going to mark this pull request "closed" because it is linked to your master. When you make the change in a branch, a new pull request will be created for you.

@Krinkle Krinkle closed this Sep 19, 2012
@s-a s-a mentioned this pull request Sep 19, 2012
@s-a
Copy link
Contributor Author

s-a commented Sep 19, 2012

Done. I Hope its ok now. the command

git branch fix-inject-swarmUrl -t upstream/master 

ended in an error message like "not a valid object".

On git pages it looks different/correct now :)

best regards

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

Successfully merging this pull request may close these issues.

2 participants