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

SCCP connection establishment and release (with fixes) #251

Closed
wants to merge 3 commits into from
Closed

SCCP connection establishment and release (with fixes) #251

wants to merge 3 commits into from

Conversation

vmykhailiuk
Copy link
Collaborator

Hello,

This PR is for issue #242.
Please see implementation notes #242 (comment)

Best Regards,
Vadim

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

Hello @vmykhailiuk

I tried to add your commits from the PR on the top of our branch "sccp-co" and failed (because of conflicts). Your latest commit in the mentioned branch is "SCCP connection reset fix". Can we check it ?

@vmykhailiuk
Copy link
Collaborator Author

Hi @vetss
I accidentally squashed into one more commits than needed. This branch contains all commits from #246 plus fixes for 7). Is this critical to merge this branch to sccp-co or new sccp-co can be created from this branch?

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

Hello @vmykhailiuk

It is not clear for me how to merge it easily... Can you provide a new PR with a proper set of changing ?

@vmykhailiuk
Copy link
Collaborator Author

@vetss
Github says that "This branch has no conflicts with the base branch". Why it's not easy to merge it?

@vmykhailiuk
Copy link
Collaborator Author

To clarify, it could be done with following commands:

$ git clone https://github.com/RestComm/jss7.git
$ cd jss7
$ git remote add vm https://github.com/vmykhailiuk/jss7.git

$ git remote -v
origin	https://github.com/RestComm/jss7.git (fetch)
origin	https://github.com/RestComm/jss7.git (push)
vm	https://github.com/vmykhailiuk/jss7.git (fetch)
vm	https://github.com/vmykhailiuk/jss7.git (push)

$ git fetch vm
$ git checkout sccp-conn-establishment-release-fixes-#242
$ git branch -D sccp-co
$ git push origin :sccp-co
$ git checkout -b sccp-co
$ git push origin sccp-co

Of course, checking out repository could be skipped if that was done earlier.

@vmykhailiuk
Copy link
Collaborator Author

vmykhailiuk commented Jul 21, 2017

@vetss
Created pull request #252 which contains the same changes as this, but is based on "SCCP connection reset fix" commit and so could be easily merged with sccp-co as it is if reusing sccp-co branch is mandatory.

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

@vmykhailiuk

just merging brings me also conflicts (:

I see at the top of PR "into RestComm:master from vmykhailiuk:sccp-conn-establishment-release-fixes-#242". May it be a problem because of your PR is based on top of "master" branch ? I am working for your updates in "sccp-co" branch now.

@vmykhailiuk
Copy link
Collaborator Author

@vetss
I see that for some reason the same commits from original branch sccp-conn-establishment-release-#242 have different id's in branch sccp-co.

sccp-co:

commit 41dc0bf0bcbcf3fb338d0923190e933827e4de41
Author: Vadim Mykhailiuk <[email protected]>
Date:   Sat Jul 8 00:25:26 2017 +0300

    SCCP connection reset fix

commit a55627ec9d37925c3073fdef4cc290dcf13e8fc1
Author: Vadim Mykhailiuk <[email protected]>
Date:   Wed Jul 5 02:02:51 2017 +0300

    SCCP connection reset

sccp-conn-establishment-release-#242

commit 13f2bdace1a9a9ed5ce6dd8568e6ad691bef6087
Author: Vadim Mykhailiuk <[email protected]>
Date:   Sat Jul 8 00:25:26 2017 +0300

    SCCP connection reset fix

commit 652555f56908e26a47d1fd5fab9267a1f5a35fa2
Author: Vadim Mykhailiuk <[email protected]>
Date:   Wed Jul 5 02:02:51 2017 +0300

    SCCP connection reset

Because of this, those branches can't be merged right now. If those commits were cherry-picked, than new commit also should be cherry-picked.

sccp-conn-establishment-release-#242

commit 5eac06c04c4317177d1deb61bde51d9d316fc216
Author: Vadim Mykhailiuk <[email protected]>
Date:   Mon Jul 10 22:24:43 2017 +0300

    Fixes

Commands to load latest changes to sccp-co branch:

$ git fetch origin
$ git remote add vm https://github.com/vmykhailiuk/jss7.git
$ git fetch vm
$ git remote -v
origin	https://github.com/RestComm/jss7.git (fetch)
origin	https://github.com/RestComm/jss7.git (push)
vm	https://github.com/vmykhailiuk/jss7.git (fetch)
vm	https://github.com/vmykhailiuk/jss7.git (push)

$ git checkout sccp-co
$ git cherry-pick 5eac06c04c4317177d1deb61bde51d9d316fc216
[sccp-co f6831be] Fixes
 Date: Mon Jul 10 22:24:43 2017 +0300
 18 files changed, 290 insertions(+), 954 deletions(-)
 delete mode 100644 sccp/sccp-api/src/main/java/org/mobicents/protocols/ss7/sccp/SccpConnListener.java

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

@vmykhailiuk

If those commits were cherry-picked, than new commit also should be cherry-picked.

yes, previous commits were cherry-picked. And I tried to cherry pick a next your commit that brought me conflicts. I can make new cherry-pick operations if you give me a list of commits to add.

$ git cherry-pick 5eac06c

Do you mean it is enouph to add a single commit from you ? I can not find that commit...

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

Or we can kill that new created "sccp-co" branch and start of working with master branch. In this case your updates needs to have a PR with differenses from master branch (may be like this one). I do not want just to merge branches to avoid of adding of unexpected commits, better to cherry-pick of concrete commits. The problem is - we need to start of code checking from some older point. If you think that your current code is more or less ready I can create a new "sccp-co" branch where we put code and from whicj you will be able to continure your work.

PS: I am using EGIT from eclipse, not just a GIT
PS2: I will try also your provided git commands

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

@vmykhailiuk

I tested your cherry-pick command "git cherry-pick 5eac06c"
and get this branch content (this is my fork so far) :
https://github.com/vetss/jss7/commits/ss7-37

Have I added all your provided code ? I am asking because a commit name is not present in this PR

@vmykhailiuk
Copy link
Collaborator Author

@vetss
Yes, you added all my latest code to https://github.com/vetss/jss7/commits/ss7-37.

@vetss
Copy link
Contributor

vetss commented Jul 21, 2017

Yes, you added all my latest code to https://github.com/vetss/jss7/commits/ss7-37.

ok, let then me check it.

@vetss
Copy link
Contributor

vetss commented Jul 25, 2017

Hello @vmykhailiuk

I accepted your commit into sccp-co branch. Check my comments in :
#242

@vetss vetss closed this Jul 25, 2017
@vetss vetss self-requested a review July 25, 2017 14:15
@vetss vetss added this to the 8.0.0 milestone Jul 25, 2017
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.

2 participants