Hi Andrew, > --- > include/dbus.h | 1 + > src/stk.c | 585 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, > 585 insertions(+), 1 deletions(-) >
Just a general comment: In the previous version of the patch you included an api document in the commit description. It might be a good idea to formalize it into a proper doc/stk-api.txt document and include it along with the patch. This way people have a much easier time understanding what is going on, and we do want feedback on the API from many people for this one. Another suggestion I have is to split out the Select Item proactive command handling from the regular Menu. Perhaps moving Select Item handling to an Agent interface along with Display Text, Play Tone, Get Input, Get Inkey, etc might be a good idea. Also, I really encourage you to split this patch apart some more. E.g. one patch for foundation of the stk atom, one part handling the Setup Menu / Menu Selection envelope, another for Select Item, etc. This makes it easier to review the code. Smaller patches are always good, and higher chance that less controversial chunks will get accepted, leading to even smaller patches in the future. Regards, -Denis Regards, -Denis _______________________________________________ ofono mailing list [email protected] http://lists.ofono.org/listinfo/ofono
