Skip to content

Asynchronous session id generation #121

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

soulman-is-good
Copy link

That's a project specific matter, but I have few of this projects that uses third-party unique generation mechanism, so I added a callback in session id generation.

Then I read #107 issue and tried to make it backward-compatible so sync functions would pass.

@dougwilson dougwilson added the pr label Feb 11, 2015
@dougwilson
Copy link
Contributor

Thanks! It looks like there may be some issue, because one of the existing tests are failing.

@soulman-is-good
Copy link
Author

My bad, missed the tests!

@dougwilson
Copy link
Contributor

Man, if you never ran the tests, one failure is pretty good.

@dwhieb
Copy link
Contributor

dwhieb commented Aug 27, 2016

@soulman-is-good Thanks for tackling this! This is something I really need for a project myself. Will you have a chance to resolve the merge conflicts anytime soon?

@derMart
Copy link

derMart commented Sep 30, 2016

would be really useful to have this...
+1

@dougwilson
Copy link
Contributor

So I'm not sure what is happening, because the changes are all over the place. There is unresolved merge conflicts I see from a question a few months ago, and a lot of the things about the PR are not backwards compatible, so would need to be deferred until the next major.

I do need to come and review this, though, to at least prepare it for the next major. @soulman-is-good are you still interested in this PR? Would you be able to re-squash it on top of the current master for me?

@soulman-is-good
Copy link
Author

Geez, seems I just f*cked up git's history here.

@dougwilson I can make another, clean and up-to-date, PR for this issue from your approval
Sorry for that.

@dougwilson
Copy link
Contributor

Weird, not sure what happened with the PR :S I just had another user with a similar and resetting the branch completely fixed it, without ever needed to make a new PR.

Assuming the mess may be from merge commits if you merged instead of rebased, you may be able to fix it with the following commands (it assumes you have this repository set as the upstream remote):

$ git checkout master
$ git fetch upstream
$ git reset --hard upstream/master
$ git checkout -f . HEAD@{1}
$ git commit -a
# Validate your commit contents; "git reset --hard c19b52d" to abort
$ git push -f # if the commit looks good

@michelcve
Copy link

What happened with this one? I still think it's a good option to be able to asynchronously generete the session id...

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

Successfully merging this pull request may close these issues.

5 participants