@alexyosifov I already removed the ones (mostly copy and pasted code) that I
found in commit ad5a346c and ac2736d6. If you need this for debugging - feel
free to add a proper debugging function that output some information at LM_DBG
log level. There is almost no performance impact in doing so,
I agree, no one should complain if commented out code gets removed at some
point and you can look up old code in git.
@alexyosifov could you please have a look at the commented out code and remove
the unnecessary functions or add TODOs, as @henningw suggested?
--
You are receiving this
I agree that it is important to have a working IMS modules and proper
integration.
About the commented out code - feel free to add a TODO or a similar comment
like this to it, then people know that it is still work in progress. Otherwise
it might get removed during integration or later, as
@henningw I've made a review on my own, before asking a second opinion. The
patch remained open for more than 10 days, which I think is too much. I wasn't
aware that you are still reviewing the patch.
The whole mode is in "work in progress" state, so I am willing to accept
commented out code
@tdimitrov I started the review yesterday as announced here, but got caught by
some other urgent topic. This is a quite large patch in the end. There was no
need to rush this into the repository.
There exists many white-space issues where the code uses a complete different
indention as the
Merged #2001 into master.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2001#event-2483541165___
Kamailio (SER) - Development Mailing
I'm merging this as it is waiting for too long time. If anyone has concerns
about the code, we'll fix them later.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@alexyosifov I will have a look this weekend to the commits.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@miconda @ngvoice
Could you please have a look at the third commit in ims_usrloc_pcscf? Seems
reasonable to me but I'm not very familiar with the code in the module, so I'll
appreciate another opinion.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
Pre-Submission Checklist
- [ ] Commit message has the format required by CONTRIBUTING guide
- [ ] Commits are split per component (core, individual modules, libs, utils,
...)
- [ ] Each component has a single commit (if not, squash them into one commit)
- [ ] No commits to README
10 matches
Mail list logo
