RestComm / jss7

RestComm Java SS7 Stack and Services
http://www.restcomm.com/
GNU Affero General Public License v3.0
177 stars 218 forks source link

SCCP connection establishment and release (with fixes) #251

Closed vmykhailiuk closed 7 years ago

vmykhailiuk commented 7 years ago

Hello,

This PR is for issue #242. Please see implementation notes https://github.com/RestComm/jss7/issues/242#issuecomment-316942268

Best Regards, Vadim

vetss commented 7 years ago

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 commented 7 years ago

Hi @vetss I accidentally squashed into one more commits than needed. This branch contains all commits from https://github.com/RestComm/jss7/pull/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 commented 7 years ago

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 commented 7 years ago

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

vmykhailiuk commented 7 years ago

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 commented 7 years ago

@vetss Created pull request https://github.com/RestComm/jss7/pull/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 commented 7 years ago

@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 commented 7 years ago

@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 <vmykhailiuk@dataart.com>
Date:   Sat Jul 8 00:25:26 2017 +0300

    SCCP connection reset fix

commit a55627ec9d37925c3073fdef4cc290dcf13e8fc1
Author: Vadim Mykhailiuk <vmykhailiuk@dataart.com>
Date:   Wed Jul 5 02:02:51 2017 +0300

    SCCP connection reset

sccp-conn-establishment-release-#242

commit 13f2bdace1a9a9ed5ce6dd8568e6ad691bef6087
Author: Vadim Mykhailiuk <vmykhailiuk@dataart.com>
Date:   Sat Jul 8 00:25:26 2017 +0300

    SCCP connection reset fix

commit 652555f56908e26a47d1fd5fab9267a1f5a35fa2
Author: Vadim Mykhailiuk <vmykhailiuk@dataart.com>
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 <vmykhailiuk@dataart.com>
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 commented 7 years ago

@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 5eac06c04c4317177d1deb61bde51d9d316fc216

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

vetss commented 7 years ago

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 commented 7 years ago

@vmykhailiuk

I tested your cherry-pick command "git cherry-pick 5eac06c04c4317177d1deb61bde51d9d316fc216" 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 commented 7 years ago

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

vetss commented 7 years ago

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

ok, let then me check it.

vetss commented 7 years ago

Hello @vmykhailiuk

I accepted your commit into sccp-co branch. Check my comments in : https://github.com/RestComm/jss7/issues/242