RIVeR-Lab / numl_val_workspace

repository for vcstool workspaces
0 stars 0 forks source link

Project Naming Convention #1

Closed sharif1093 closed 8 years ago

sharif1093 commented 8 years ago

Currently only the name of the repos follow the convention of having numl_ as a prefix. I think it would be easier to force this convention to subprojects as well, so that we can take advantage of text completion after numl_. Some subprojects that currently do not follow this naming convention:

That might be changed to:

Note removal of word valkyrie as it adds no information about the project, or addition of _demo__ and _manipulation__ to place all demo and all manipulation projects together in the text completion list.

velinddimitrov commented 8 years ago

Please place any thoughts on this here. I think it is a good idea and support renaming/refactoring.

murphywonsick commented 8 years ago

Ya I like the idea of prefacing all the package names with their repository. It does make for long names though, which I don't particularly care for, but overall it should make organization and finding things much easier when in the terminal. So I guess I support this, unless there is a better idea.

Side note: I know you are just giving an example, but I just think python_tutorials should definitely stay python_tutorials and not be renamed to demonstration. So it would refactor to numl_val_demo/numl_val_demo_python_tutorials. The word tutorials is much more clear in my opinion for people wanting to find easy code for learning.

sharif1093 commented 8 years ago

Yes, the point about python_tutorials is more kind of example. However, a name like numl_val_demo_pytutorials is smaller and more consistent with other ROS packages.

I have another concern about package names. Packages that live in different branches may have the same names that may render merging those branches a very tough job. So I recommend that anybody who creates a package in any of the branches of any of the repos, registers its name and a short description of that package in a wiki page, let say a wiki page in numl_val_workspace, so that redundant package names are not created in the first place, and if another package with almost the same goal exists in advance, people try to extend it rather than creating their own new package. Needless to say, the idea of development in branches can still be encouraged, but this exercise makes merging branches an easier task.

Murphy, it would be great if you add whichever package naming convention that we decide is best, to the Code Style Guide Wiki Page.

allspawj commented 8 years ago

I agree with the original proposal for changing all package names to prepend the repository they are in.

Regarding tutorials, I intended that package to contain very brief, minimum required code samples for controlling valkyrie. I think tutorials better describes it intended purposes than something like demonstrations. I do however intend to add c++ tutorials as well shortly, I am leaning on renaming python tutorials to something like numl_val_demo_tutorials.

If anyone has any dissenting opinions or other thoughts to weight in, please do so. Otherwise I will push the changes.

allspawj commented 8 years ago

About package names, we can just push for people to make an empty package with a README stating what the goal of the package is. Push that to develop, then branch and start your coding. Requiring people to consult an external source to check if their name is taken seems prone to people just not bothering to check, Especially given how unlikely it is, and how easy it is to refactor if a conflict happens.

murphywonsick commented 8 years ago

I'll definitely add the package naming convention to the wiki once we close this issue.

About package names being the same...I vote we don't worry about this. I agree with Jordan, the likelihood of two packages being named exactly the same is very low, no one is going to check the wiki, and refactoring is super easy if a conflict does occur. However, I think we should avoid asking people make an empty package with a README and push to develop. It is just going to introduce ways to break the develop branch and then one of us (AKA Jordan) is going to have to fix one :stuck_out_tongue_winking_eye:

sharif1093 commented 8 years ago

Editing the README.md for each repo can be done online in a more convenient way, and people might be encouraged to do so on the github itself to avoid unintended pushes to the develop branch.

Regarding the name of the tutorial packages, pytutorials and cpptutorials might be better names for that purpose, reflecting the type of tutorial people would expect in those packages.

Renaming package names, not very similar to other types of refactoring, needs changing it wherever it appears in package.xml and CMakeLists.txt. Also it may be needed to rename them in the launch-files that call those packages. I don't know more efficient tools for this purpose (renaming packages) other than sed or awk. However, my preference is to avoid such cases from the beginning when the project structure is still not too complicated.

allspawj commented 8 years ago

I don't have a problem with people adding empty packages to the repo with a readme, an empty package won't fail to build. It also explicitly states what you are working on so someone else who might be doing the same thing has a heads up.

I don't see an advantage for splitting the tutorials package into two different packages. Can just have tutorials with scripts/pythonuttorials and src/cpptutorials. That said, don't have a particularly strong opinion on this one package vs two.

It's pretty unlikely two people choose the same name for a package, and will likely only possibly occur in the demo repository where many people might have different demonstrations for walking or something. Once we get rolling people should be syncing with git often (daily or weekly at latest if developing). You shouldn’t ever find yourself in a case where a new package containing months of work is suddenly conflicting with someone else's code.

allspawj commented 8 years ago

Refactoring is done. I also added a catkin ignore for the package I incorrectly pushed using the new api. My badddd.