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.

Sign up for GitHub

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

Jump to bottom

fix (proxy): Default to karma port and host #1007

Merged
merged 1 commit into from
Apr 8, 2014
Merged

fix (proxy): Default to karma port and host #1007

merged 1 commit into from
Apr 8, 2014

Conversation

deepak1556
Copy link
Contributor

Closes #872

@deepak1556
Copy link
Contributor Author

\cc @vojtajina

aymericbeaumet
aymericbeaumet reviewed Apr 8, 2014
View reviewed changes
lib/middleware/proxy.js
proxyConfig[proxyPath].port = proxyConfig[proxyPath].https ? '443' : '80';
proxyConfig[proxyPath].port = !proxyConfig[proxyPath].host ? (
(proxyConfig[proxyPath].host = constant.DEFAULT_HOSTNAME) && constant.DEFAULT_PORT)
: proxyConfig[proxyPath].https ? '443' : '80';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be easier to read if you'd split this code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@vojtajina
Copy link
Contributor

Thanks @deepak1556 !!!
Can you squash both commits into a single one?

@vojtajina
Copy link
Contributor

Otherwise LGTM.

@deepak1556
Copy link
Contributor Author

Fixed and squashed, Thanks @aymericbeaumet and @vojtajina. Ahh damn i messed up the commit message, is there a way to fix just tat ?

deepak1556 added a commit that referenced this pull request Apr 8, 2014
@deepak1556
Merge pull request #1007 from karma-runner/fix-proxy
337a538 
fix (proxy): Default to karma port and host
@deepak1556 deepak1556 merged commit 337a538 into master Apr 8, 2014
@deepak1556 deepak1556 deleted the fix-proxy branch April 8, 2014 23:39
@aymericbeaumet
Copy link
Member

I'm afraid that everything's pushed is definitely pushed :/

@deepak1556
Copy link
Contributor Author

--amend did the trick!

@aymericbeaumet
Copy link
Member

Well git commit --amend allows to edit the commit locally.

You will not be able to push it without --force.

@deepak1556
Copy link
Contributor Author

ahh yeah tats a bad way though bt its not a problem if no one is working on the same branch, i guess.

@aymericbeaumet
Copy link
Member

That's not a problem from the moment nobody's fetched your work.

@vojtajina
Copy link
Contributor

Force push to your branch is OK. Just don't force push to master/stable branches.

@deepak1556
Copy link
Contributor Author

👍

@jeremyroberts0 jeremyroberts0 mentioned this pull request Mar 12, 2015
Open
@metamatt metamatt mentioned this pull request Jun 29, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

There should be a way to ask karma to serve a specific directory in a given path
3 participants
@deepak1556 @vojtajina @aymericbeaumet