ghitchens / cell-tool

Utility for managing nerves devices running nerves_cell
Other
9 stars 0 forks source link

Please consider to add a global namespace (e.g CellTool) as a prefix for all the modules #3

Open milmazz opened 9 years ago

milmazz commented 9 years ago

Please consider to put CellTool as a prefix for all the modules, that way to avoid to use another's package namespace, Hex include a naming policy for those cases.

CellTool
Cmd.Denormalize
Cmd.Discover
Cmd.Help
Cmd.Inspect
Cmd.Ip
Cmd.Normalize
Cmd.Provision
Cmd.Push
Cmd.Reboot
Cmd.Test
Cmd.Watch
Finder
Inet
Jrtp
SsdpClient
ghitchens commented 9 years ago

Thanks for your proposal. I agree in principle, however in this case cell-tool is under fairly heavy development, it's a utility script tool, and not a library at this time, and will not be distribute by hex. I will consider namespacing anyway.

Is it your impression that the namespacing is important even for escript tools that aren't distributed by hex?

milmazz commented 9 years ago

@ghitchens The reference to Hex's naming policy was just a reference, but I really recommend to maintain a common namespace. For example, in ExDoc we keep ExDoc as a global namespace and we resemble the directory structure when we define a new module, this means that in the file lib/ex_doc/formatter/html.ex we define a module called: ExDoc.Formatter.HTML and so on, we also try to resemble this structure under tests, that way is easy to find the unit tests file related with the previous module: test/ex_doc/formatter/html_test.exs will contain the module ExDoc.Formatter.HTMLTest. Also, you can find that pattern in the Elixir source code.

As a side note, ExDoc is also an utility script tool and it's available via hex.pm, so, you can execute ExDoc with something like this:

$ ex_doc "PROJECT_NAME" "PROJECT_VERSION" path/to/project/ebin

But also ExDoc offer a series of Mix tasks. I don't know if in CellTool's case could be an improvement to offer some tasks like this:

$ mix celltool.ip <params>
$ mix celltool.denormalize <params>
$ mix celltool.watch <params>

If you agree, I can work on this, with Perl and a couple of regexs this can be done easily.

ghitchens commented 9 years ago

I agree in general about the namespacing, - I am doing some work on cell tool and doing some hand merges of code from a private project, I will address namespacing as I do that merge over the next few days.

I'm not sold yet on the ability of invoking cell-tool via mix tasks, so I'm not inclined to head that direction at this time.

milmazz commented 9 years ago

@ghitchens No problem, the Mix tasks was just an idea, let me know if I can help with something else.

ghitchens commented 9 years ago

@milmazz appreciate your help and suggestions. I'm a bit new to open source projects so getting down conventions and practices helps a lot.

ghitchens commented 9 years ago

@milmazz can you look and see if this is closer to what you were hoping for? I discussed with justin and we are thinking we might have multiple cli's of which cell is one... so we chose Nerves.CLI.Cell namespace.

ghitchens commented 9 years ago

Refactored directories to match as well, can you check to see if I'm following best practices on refactoring of directories? Thanks for all your help! cell-tool was one of the first elixir projects I worked on and I definitely didn't have proper conventions down!

I am planning on moving repo to nerves_cli_cell, as well, let me know if you see any issues there.

milmazz commented 9 years ago

First of all, thanks for take this issue into account.

I don't see why CLI needs to be in the Nerves.CLI.Cell namespace, at least for me the only section that is actually a CLI (Command Line Interface) right now is located in lib/cell.ex (I'll explain later why I recommend to move this into lib/cell/cli.ex).

With a little extra work, we can change the functions under lib/cell/cmd directory and allow other applications can use it or extend their behavior. Suppose that someone wants to reuse this application and create a Web interface.

What do you think about the following structure?

File Module
lib/cell.ex Nerves.Cell
lib/cell/cli.ex Nerves.Cell.CLI
lib/cell/finder.ex Nerves.Cell.Finder
lib/cell/jrtp.ex Nerves.Cell.JRTP
lib/cell/cmd/normalize.ex Nerves.Cell.Cmd.Normalize
lib/cell/cmd/push.ex Nerves.Cell.Cmd.Push
test/cell_test.exs Nerves.CellTest
test/cell/cli_test.exs Nerves.Cell.CLITest
test/cell/finder_test.ex Nerves.Cell.FinderTest
test/cell/jrtp_test.ex Nerves.Cell.JRTPTest
test/cell/cmd/normalize_test.ex Nerves.Cell.Cmd.NormalizeTest
test/cell/cmd/push_test.ex Nerves.Cell.Cmd.PushTest

As I mention before, the content of your lib/cell.ex file normally[1] goes into lib/cell/cli.ex, which holds the command line parser for your application.

I agree with your approach of keep everything under the global namespace Nerves, actually you can see the same approach in the Phoenix Framework.

[1] Two examples on top of my head right now are: ExDoc (lib/ex_doc/cli.ex), Earmark (lib/earmark/cli.ex). Also, the Programming Elixir book by Dave Thomas mention this.

@ghitchens please let me know what do you think. HTH

ghitchens commented 9 years ago

@milmazz - Thanks again.

I see your point, agree, and this could be an improvement (allowing this "cell utility" functionality to be used via a web interface, for instance), although I don't like the factoring above.

There is already going to be Nerves.Cell which which encapsulates a lot of the cell-related functionality as a library. These are features that implement the behavior of a cell, rather than utilities or CLI that manipulate cells (like this repo). nerves_cell is not yet in the nerves repo but coming soon.

Nerves.CLI.Cell was intended to be a CLI for "managing cells" only, but I think it is a great idea to break it apart a bit long term. I'm not sure if it should be one library and a CLI or more than one library, but I'm thinking more likely it should be more than one.

For instance, I agree some of the functionality should be refactored in the future as Nerves.CellFinder or Nerves.CellClient so they can be reused as you suggest. I hadn't thought of starting from this project to do that, but it's as good a starting point as any.

I am not currently going to focus on this personally as I need to get some of the other pieces into place rather than continue to evolve this project, but if you are interested in working on this I'm absolutely thrilled to have you work on it.