Tunnel integration #184

Closed
wants to merge 20 commits into
from

Conversation

Projects
None yet
4 participants

darul75 commented Feb 22, 2014

Here is a new branch for merge, tells me if it fits better. It rollbacks all my doc/package name updates.

See below for changes in code, it is much cleaner.

You can test in your environment with mocha tests/oauth-tunnel.js

By default you will receive this message by running test

"message":"Bad Authentication data","code":215

but by putting your real twitter keys, you will see God !!

darul75 commented Feb 23, 2014

do you think it is correct or should I create another branch ?

@darul75 darul75 referenced this pull request Feb 24, 2014

Closed

proxy tunnel integration #173

darul75 commented Feb 24, 2014

Look in "Files changes", only change I see now is tunnel dependency add. But maybe I am still wrong.

"dependencies": {
"tunnel": "0.0.2"
}

@darul75 yah sorry, I see that now. Just a lot going on as far as commits.

darul75 commented Feb 24, 2014

But you are right, I had to revert many things because I was thinking to make my own repo few month ago as I did not get any feedback from lib author ;) And I started renamed it etc...but all is revert now normally. I have made many tests and not seen any big mistake.

Any update on this PR ?

Owner

ciaranj commented Sep 26, 2014

Errr, I've not had chance to look at it :/ But a brief look at it suggests I won't pull it in , in the current state, far too many commits and changes upon changes :/ Needs some re-work before consideration I'm afraid :(

darul75 commented Sep 26, 2014

if you want I can try redo the PR as it was not very cleaned when I pushed it.

Owner

ciaranj commented Sep 26, 2014

It seems popular so it may be worth the effort , but I would want the tunnel dependency to be 'optional' :)

darul75 commented Sep 26, 2014

yes I understand, what are your propositions to achieve it then

Owner

ciaranj commented Sep 26, 2014

Hmm, I have none :/ Certainly putting it in the package.json as an optional dep, and making the code execute conditionally if the module is present or not would be one approach ?

darul75 commented Sep 27, 2014

it looks like a nice solution, I will refactor PR as soon as my girlfriend let me free time ;)

@desmondmorris desmondmorris referenced this pull request in desmondmorris/node-twitter Sep 27, 2014

Closed

Support for corporate proxy ? #10

@darul75 darul75 closed this Dec 23, 2014

darul75 added a commit to darul75/node-oauth that referenced this pull request Dec 23, 2014

darul75 added a commit to darul75/node-oauth that referenced this pull request Dec 23, 2014

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