On 12/8/12 1:36 PM, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

you patch changes the values for server and user-agent value in config.h.

sorry, that was my mistake. i'll fix it.

...

Could be better to split such large commits (when there are internal api changes) on affected components, because it is easier to look at the patches (e.g., doing one commit for each affected module and one for the core) -- not a rule, just an observation based on past experiences.

if i had spit it, people would have complained that master is not compiling. i would not have likes someone to do that to me.

You can commit locally and push all at once, so practically all of them will be available on remote repository at the same time. I did it many times (it is actually the usual way because it easier to review later or send only specific links to the authors of the components I want to be reviewed).

Cheers, Daniel

On 12/8/12 1:55 PM, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

You can commit locally and push all at once, so practically all of them will be available on remote repository at the same time. I did it many times (it is actually the usual way because it easier to review later or send only specific links to the authors of the components I want to be reviewed).

this was all or nothing change. making several commit locally would just meant more work for me.

I think this is not really a standing argument in the overall context. It is very important to keep robustness of the application, meaning that when one affects the components of other developers, it has to be an easy way for them to spot the changes.

It is just about running several commit commands instead of one, the push to remote is one command. What would change is the subject saying what component was affected. If there is no proper time at the moment for something, better don't rush it, could be harder later to sort it out.

rather than argue about administrative things,

I don't think this is an administrative thing, it is related to development. Again, it was a suggestion. I did it because it happened to bring in an invalid change. It was not an enforcement. Looking at very large commits require also lot of time, usually has to be done in one time frame. Smaller commits can be reviewed easier sequentially, at different points in time.

i would like to get comments on this, because it needs to be done one way or the other next:

Well, the master branch doesn't compile, pv module throws errors.

Also, the modules in modules_s have to be made to compile for the moment, otherwise master branch compilation fails anyhow. They may be removed, but that will still take time, it will not happen in the next hours.

...

if we do that, then for consistency, get_branch and next_branch should be dropped altogether and replaced by get_sip_branch (which already exists) and get_next_sip_branch (which would be new).

so either new arguments to get_branch and next_branch or replace them with get_sip_branch and get_next_sip_branch.

It is no really need to replace them (the old ones). You can add a new function returning the pointer to the next branch and use it. For long term, I think the one with the pointer is easier to maintain whenever we need to add new members in the structure, I would go for that.

However, the old ones can be kept, so not much code is affected, probably not all places where get_branch/next_branch are used need the instance/reg-id.

Cheers, Daniel