17 May
2009
17 May
'09
5:13 p.m.
hi holger,
these patches are not the "final" patch. some things must be discussed before of
yourse. i removed the out commented code from the patch. some comments like "// todo
..." are not removed. when you look at the file
http://home.eversberg.eu/stage1.html, you will see the new version. whenever there is
something to complain about, i will change it, until we have patches that all of us
agree.
please ask if you don't fully understand the patch. especially for the
BSC<->application interface, i was too lazy to describe all details. you will find
mncc.c usefull to see how the interface is handled.
regards,
andres
-----Ursprüngliche Nachricht-----
Von: openbsc-bounces(a)lists.gnumonks.org [mailto:openbsc-bounces@lists.gnumonks.org] Im
Auftrag von Holger Freyther
Gesendet: Sonntag, 17. Mai 2009 09:35
An: openbsc(a)lists.gnumonks.org
Betreff: Re: patches for openbsc
On Friday 15 May 2009 12:37:50 Andreas.Eversberg wrote:
hi,
finally my patch is documented (almost). you will find the
documentation of the patch:
http://home.eversberg.eu/stage1.html
the patch in plain text fromat:
http://home.eversberg.eu/stage1.patch
it also includes the changes i posted here before. if you have
questions, ask.
Hi Andreas,
thank you for the patches. Let me start to review the patches one by one as replies to
this one. One general thing with this and the previous patch. I'm quite picky when it
comes to commented out code, and left overs from debugging.... please try to avoid leaving
that in.
z.
18 May
18 May
6:14 a.m.
New subject: patches for openbsc
On Sunday 17 May 2009 17:13:24 Andreas.Eversberg wrote:
hi holger,
these patches are not the "final" patch. some things must be discussed
before of yourse. i removed the out commented code from the patch. some
comments like "// todo ..." are not removed. when you look at the file
http://home.eversberg.eu/stage1.html, you will see the new version.
whenever there is something to complain about, i will change it, until we
have patches that all of us agree.
please ask if you don't fully understand the patch. especially for the
BSC<->application interface, i was too lazy to describe all details. you
will find mncc.c usefull to see how the interface is handled.
Sorry,
we don't get anywhere with this. How should I comment on the stage1.html? Send
you a diff against html? Currently it is way too much work to pick the good
parts by hand and leave the other stuff out, I would really welcome patches one
could just apply.
Examples:
1.) Installation of the lib...
NO! Not via an install-data-hook... If you want the lib to be installed change
it away from a noinst lib. This should honor the --prefix one is giving when
configuring... this is only leaving the includes...
And please don't mix changing the noinst to a proper library and adding new
files... They have nothing to do with each other...
2.) ... Add mncc.c in one go and not by sprinkling changes around...
3.) Adding the timer.
The change looks fine, we will need to test it on real hardware... BUT it is
depending on your rename of the timer functions introduced later... maybe it
is time for using git and git-format-patch? So yes, patch is okay, but we can
not just apply it because of its dependencies (timer rename, new trau include,
...) It would also be appreciated if you can rediff this part as the debug
statements have been added... And you have unrelated changes like the
autorelease within this patch, something I have merged to openbsc by hand
already...
And with the change of input/misdn.c you try to sneak in a mISDNuser
dependency without changing the configure.in. I already asked why is this
needed? What is the benefit?
4.) Skipped for now
5.) Okay, should be something like the new 3rd patch as 3rd) is depending on
this one. All fine... but the title of the patch does not match the content.
The renaming has nothing to do with the change of semantic of this function.
So we need one patch for renaming the methods (to not pollute the global
namespace...) + users of it and one for changing the semantic + user of it.
6.) Skipped for now
7.) Don't you miss a change to include/openbsc/gsm_data.h? Currently the bts
is passed in terms of the data (or within a struct for data), can't this work
for for you?
sorry, need to stop here right now, I see most of your work is in the NMCC
change... so for now what about trying to get 1-7 resolved and progress on
these items?
z.
3:22 p.m.
New subject: patches for openbsc
On Mon, 18 May 2009 06:14:03 +0200
Holger Freyther <zecke(a)selfish.org> wrote:
we don't get anywhere with this. How should I
comment on the
stage1.html? Send you a diff against html? Currently it is way too
much work to pick the good parts by hand and leave the other stuff
out, I would really welcome patches one could just apply.
There's a wiki. Why don't you just write all the stuff into it? Way
easier to work on.
cheers,
--
codec
19 May
19 May
4:18 a.m.
New subject: patches for openbsc
Dear Andreas,
first of all, as I indicated in the past, I am very thankful for the amount
of work you are committning to OpenBSC. So please always keep this in mind!
Dear Holger,
we all bring different skills to this project. Andreas' strength is that
he has a lot of knowledge of traditional ITU-T/ETSI telephony protocols,
something that we clearly don't have. Apart from all his work, he actually
also documented his work in detail - even if in a method quite uncommonly
seen in FOSS projects for development discussion (colorful HTML on his
website).
You on the other hand (like me) have extensive experience in participating in
large FOSS projects that follow certain rules and 'best practise' when it comes
to the structure of contributions.
Andreas is trying to help us with his code, so please be friendly and welcoming
to him. I agree with you that the mode of submitting one large patch rather than
an incremental per-feature patchset is not how contributions are normally acceptd.
I also agree that cosmetic/syntax changes need to be kept separate from actual
semantic changes of the code, or that autotools should be used the "right" way -
but we can communicate that in a nicer way.
Andreas, I hope you will bear with us and go through the process of splitting
up your patches. I know it is the least exciting part of anyone's work, but I
believe it is an important skill to have when working within the FOSS community.
What I would like to see is independent and incremental patches in the
following order:
* timer rename throughout the existing code
* llist header changes (why were they made? c++?)
* the transmit delay timer
* introduction of trau interface
* bsc_select_main(polling)
* paging extension with cbfn BTS pointer
* the actual MNCC interface
* installation of libbsc (this should be last, since before the other
changes it is of no use)
after applying each of those patches individually, the code should still
compile and work. Each patch should be sent as an independent patch/diff
file to the openbsc-devel list, including a short description on top of the
patch what it does.
If submitted this way, I can agree to merge all the infrastructure work right
away, i.e. the above list up to and including the 'paging extension'. This
leaves Andreas' stage1 patch limited to the MNCC interface, which I first want
to find some more time to review and play with it.
Is that something we can all agree to?
Thanks.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
7:01 a.m.
New subject: patches for openbsc
On Tuesday 19 May 2009 04:18:57 Harald Welte wrote:
Dear Andreas,
first of all, as I indicated in the past, I am very thankful for the amount
of work you are committning to OpenBSC. So please always keep this in
mind!
Dear Holger,
we all bring different skills to this project. Andreas' strength is that
he has a lot of knowledge of traditional ITU-T/ETSI telephony protocols,
something that we clearly don't have. Apart from all his work, he actually
also documented his work in detail - even if in a method quite uncommonly
seen in FOSS projects for development discussion (colorful HTML on his
website).
You on the other hand (like me) have extensive experience in participating
in large FOSS projects that follow certain rules and 'best practise' when
it comes to the structure of contributions.
Andreas is trying to help us with his code, so please be friendly and
welcoming to him. I agree with you that the mode of submitting one large
patch rather than an incremental per-feature patchset is not how
contributions are normally acceptd. I also agree that cosmetic/syntax
changes need to be kept separate from actual semantic changes of the code,
or that autotools should be used the "right" way - but we can communicate
that in a nicer way.
Dear Andreas,
sorry about the tone. I'm very interested in getting your changes into the
OpenBSC repository as they are great improvements. My mail was not meant to be
rough but was meant as a way forward to get things merged before you end up
with a huge pile of changes (this was probably the reason that harald and me
started to merge bits and pieces by hand as well). I sincerely agree with
Haralds proposed order of changes and please let us progress this way.
z.
PS: Sorry, I went to bed early here, I try to reach you tonight
Some more bla bla on why small chunks of changes.
First of all we have all limits, by having smaller changes we are better to
judge this individual change, the impact and such without reaching the limit.
Having small independent changes allows us to roll-out (revert) a particular
change (e.g. when our judgment was wrong) without ending in a big manual merge
catastrophe. And finally by having each change compilable we can fall back to
things like git bisect.
5462
days inactive
5464
days old
4 comments
4 participants
participants (4)
-
Andreas.Eversberg -
codec -
Harald Welte -
Holger Freyther