internet-apps

internet-apps

[Tommaso Pecorella, 2015-11-03 22:42:46]

Proposed split between "applications" module and "internet-apps" module (the
module name is temporary, if you have a better one, feel free to propose it).

As mentioned in previous mails, the goal of the new module is to host *only*
TCP/IP specific applications (or demons), like ping, radvd, dhcpd, etc.

The apps in this module are bound to be internet-specific, and including this
module will result in a strict and explicit dependency on the internet module.

[Peter Barnes, 2015-11-04 18:32:01]

Two Q:

Does Rietveld not track file moves?  Pity

Several files have seemingly trivial reordering of includes, typically moving a
helper to the top of the list.  Are these changes necessary? (Asking for my
edification)

[Tommaso Pecorella, 2015-11-04 22:15:42]

On 2015/11/04 18:32:01, Peter Barnes wrote:
> Two Q:
> 
> Does Rietveld not track file moves?  Pity

It does but in a strange way. if you click the file it will say "renamed".
Counterintuitive.
Example:
Index: src/internet-apps/helper/ping6-helper.h
===================================================================
rename from src/applications/helper/ping6-helper.h
rename to src/internet-apps/helper/ping6-helper.h


> Several files have seemingly trivial reordering of includes, typically moving
a
> helper to the top of the list.  Are these changes necessary? (Asking for my
> edification)

It's totally unnecessary, and believe me: I didn't move any of these includes.
Eclipse did it for me. It seems that there's some rule saying that that header
include should be first, but there's also someone arguing the exact opposite.
Anyway, I don't think we have a strict rule yet, and Eclipse is keen to
implement Google's one (see for  example
http://www.ostricher.com/2014/11/sort-your-include-files/).

if needed I can revert these changes, as they're not really needed, and leave
them for when we'll do a global headers reordering and cleanup.

[Tom Henderson, 2015-11-05 20:22:11]

I am fine with the 'internet-apps' name; can't propose one that I like better.

Other nits to pick up:

- SetGroupName () needs to be changed on all TypeIds
- doxygen "\ingroup applications" needs to be updated throughout the header
files

we need an 'internet-apps.rst'; suggest to use the template in create-module.py
and fill it in as best as you can

we need a default maintainer in bugzilla for a new 'internet-apps' product
(Tommaso?)

https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript
File src/internet-apps/wscript (right):

https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript#new...
src/internet-apps/wscript:24: #     'test/internet-apps-test-suite.cc',
what ought to be tested in this module?

https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript#new...
src/internet-apps/wscript:41: #     bld.recurse('examples')
are there any examples to move to this module (perhaps from examples/
directory)?

https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript#new...
src/internet-apps/wscript:43: # bld.ns3_python_bindings()
should be uncommented

[Tommaso Pecorella, 2015-11-05 23:13:42]

On 2015/11/05 20:22:11, Tom Henderson wrote:
> I am fine with the 'internet-apps' name; can't propose one that I like better.
> 
> Other nits to pick up:
> 
> - SetGroupName () needs to be changed on all TypeIds
> - doxygen "\ingroup applications" needs to be updated throughout the header
> files

Done and done

> we need an 'internet-apps.rst'; suggest to use the template in
create-module.py
> and fill it in as best as you can

Done. We could write a lot more, but I don't think it's a pro right now.
Moreover, people should know what's a ping...

> we need a default maintainer in bugzilla for a new 'internet-apps' product
> (Tommaso?)

I felt it coming... ok, I'll take it.
 
> https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript
> File src/internet-apps/wscript (right):
> 
>
https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript#new...
> src/internet-apps/wscript:24: #     'test/internet-apps-test-suite.cc',
> what ought to be tested in this module?

We should write unit testing for each app tbh. Something to be done (in the
future).

>
https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript#new...
> src/internet-apps/wscript:41: #     bld.recurse('examples')
> are there any examples to move to this module (perhaps from examples/
> directory)?

A lot of examples, but I wouldn't move them right now. Users are already NOT
checking the "examples" directory, and they're even less keen in checking an
example directory embedded in a module.
We could do *new* examples (perhaps more interesting) for the outer examples
directory and move the simpler ones in the module. However, removing something
without a replacement isn't a good idea.

>
https://codereview.appspot.com/275090043/diff/1/src/internet-apps/wscript#new...
> src/internet-apps/wscript:43: # bld.ns3_python_bindings()
> should be uncommented

Done.