I fixed something in SCCP_Emulation.ttcn (from Ericsson), and trying out what I'm allowed to do, it happened so that I just pushed the fix onto https://github.com/osmocom/titan.ProtocolEmulations.SCCP master
See https://github.com/osmocom/titan.ProtocolEmulations.SCCP/commit/17a894fc6620...
The fix is trivial enough, but I'd like to note that I pushed something that no-one reviewed, and hope that's ok.
my github user is part of that Osmocom group, I set the remote to a git@... URL, so that's why I could just push onto master.
For the future, do we have a process to review fixes for Ericsson's ttcn code? Send it to this ML?
~N
Hi Neels,
On 16.04.2023 06:37, Neels Hofmeyr wrote:
I fixed something in SCCP_Emulation.ttcn (from Ericsson), and trying out what I'm allowed to do, it happened so that I just pushed the fix onto https://github.com/osmocom/titan.ProtocolEmulations.SCCP master
Seehttps://github.com/osmocom/titan.ProtocolEmulations.SCCP/commit/17a894fc6620...
The fix is trivial enough, but I'd like to note that I pushed something that no-one reviewed, and hope that's ok.
the patch looks good to me. As I said in the IRC, make sure to update the 'titan.ProtocolEmulations.SCCP_commit' in osmo-ttcn3-hacks.git, so that `make deps` will pull your new commit.
my github user is part of that Osmocom group, I set the remote to a git@... URL, so that's why I could just push onto master.
Now that you mentioned GitHub, where I have no access to anymore, I am wondering if we should host titan.* forks on Osmocom's own Gitea instance rather than on a politically affiliated service like this one? If we agree on that, I could take care of the migration.
For the future, do we have a process to review fixes for Ericsson's ttcn code? Send it to this ML?
IMO, ideally we should try submitting fixes upstream (to [1] in this case), so that the TITAN maintainers can be involved in doing code review too. They're generally open for contributions and already accepted lots of patches from Harald.
Worth mentioning that we have titan.ProtocolModules.BSSMAP in Gerrit, but not titan.ProtocolEmulations.SCCP. We could add it to Gerrit, but I doubt we will see such patches too often, so probably not worth adding.
I am fine with sending patches to this ML and reviewing them here. If we go the Gitea route, we could use its pull request feature.
Just thinking out loud, let's see what others think.
[1] https://gitlab.eclipse.org/eclipse/titan/titan.ProtocolEmulations.SCCP
Best regards, Vadim.
Hi Vadim, Neels,
On Sun, Apr 16, 2023 at 07:37:08AM +0700, Vadim Yanitskiy wrote:
the patch looks good to me. As I said in the IRC, make sure to update the 'titan.ProtocolEmulations.SCCP_commit' in osmo-ttcn3-hacks.git, so that `make deps` will pull your new commit.
... and first and foremost, make sure that the fix is submitted to upstream. The process for that has changed several times, and was not the same for all TITAN repositories: Some were using gerrit, and others github. I believe now all are in gitlab, but I'm not following closely
Now that you mentioned GitHub, where I have no access to anymore, I am wondering if we should host titan.* forks on Osmocom's own Gitea instance rather than on a politically affiliated service like this one? If we agree on that, I could take care of the migration.
The reason for hosting those repos on github was that upstream TITAN requested pull-requests to be placed there for some of their repositories. If that is still the case, we should cotinue to have our repo where upstream wants PRs.
If upstream has now migrated to somewhere else, then our repo should be from wherever PRs can be posted.
IMHO, there's no point in having osmocom forks of those TITAN repositories in another place from where we cannot contribute upstream.
IMO, ideally we should try submitting fixes upstream (to [1] in this case), so that the TITAN maintainers can be involved in doing code review too.
yes, that is the normal process. The osmocom forks normally only exist temporarily until fixes are merged upstreams - or in exceptional cases where we have code that cannot be accepted upstream.
They're generally open for contributions and already accepted lots of patches from Harald.
Not just by me, there were other contriubtions from osmocom developers, too.
Worth mentioning that we have titan.ProtocolModules.BSSMAP in Gerrit, but not titan.ProtocolEmulations.SCCP. We could add it to Gerrit, but I doubt we will see such patches too often, so probably not worth adding.
I'm surprised that any of it is in our gerrit. That probably was an error or oversight, and should not be repeated.
I am fine with sending patches to this ML and reviewing them here. If we go the Gitea route, we could use its pull request feature.
I'd say follow whatever upstream process and post a link here, so osmocom developers can review it in upstream. Reviewing only at osmocom before contributing upstream makes only sense for large/complex changes where the author doesn't feel confident sending them upstream straight away - which doesn't appear to be the case here, where it's a rater trivial/obvious fix?
Regards, Harald
-
Harald Welte -
Neels Hofmeyr -
Vadim Yanitskiy