Project

General

Profile

Task #2839

make module and file naming consistent

Added by Mark Abraham 3 months ago. Updated 16 days ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
core library
Target version:
Difficulty:
uncategorized
Close

Description

Our src/gromacs modules are named inconsistently when the name has more than two words. The "complexname" pattern can be hard to read, but is somewhat consistent with class naming. The "complex_name" pattern is familiar and is preferred at https://google.github.io/styleguide/cppguide.html#File_Names. The "complex-name" pattern works and saves a shift key press. The "ComplexName" pattern works but might be tricky if we would ever again encounter a non-case-sensitive file system (albeit that we already use case in naming short-range kernels and reference data files).

Names of files within modules should also follow a particular pattern. We should avoid short file names likely to be present in multiple modules (logging.h, util.cpp, etc.). Once a module has a conforming design, then the file names should anyway relate to the class being implemented in them. In some cases that will mean the module name is a prefix for the class name, and thus file name. So we should expect to find the NbnxmGridSearch class implemented in src/gromacs/nbnxm/nbnxm_grid_search.cpp.

Thoughts?

Associated revisions

Revision 90aa21a7 (diff)
Added by Roland Schulz 3 months ago

Rename all source files from - to _ for consistency.

Refs #2839

Change-Id: I3a1af667b2b266b1fd65586445b5e7a8a1b6b618

Revision 0c01697c (diff)
Added by Eric Irrgang 17 days ago

Remove stale "TODO".

Refs #2839

Change-Id: I927ee73fc9c6ffa423d37fd4606654392cb8a1b8

History

#1 Updated by Mark Abraham 3 months ago

At https://gerrit.gromacs.org/#/c/9021/, Mark preferred consistency, then readability, and doesn't care what we choose. Also there, Roland preferred that we use only one of dash and underscore and observed that there are names including acronyms that are particularly hard to read. At https://gerrit.gromacs.org/#/c/8940/, Szilard preferred underscores. Berk didn't express an opinion. Erik's kernels have had both underscore and case, so he'll cope.

That sounds like the "complex_name" pattern has broad support for module naming.

For naming files within modules, are we OK with things like src/gromacs/listed_forces/gpu_bonded_impl.cu?

I'll put together an updated entry in the style guide if people give me a bit of feedback.

#3 Updated by Gerrit Code Review Bot 3 months ago

Gerrit received a related patchset '4' for Issue #2839.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I3a1af667b2b266b1fd65586445b5e7a8a1b6b618
Gerrit URL: https://gerrit.gromacs.org/9076

#4 Updated by Eric Irrgang 17 days ago

Is there still an open request for feedback here?

Case is bad.
Underscores are clear (except when they typeset funny)
Things like src/gromacs/listed_forces/gpu_bonded_impl.cu? look right.

I don't see the need to prefix module name to header file name. The path is part of the file name except in a handful of global headers and where the header is part of the current module. Does it mess up the source checker scripts or something?

#5 Updated by Szilárd Páll 16 days ago

Eric Irrgang wrote:

The path is part of the file name except in a handful of global headers and where the header is part of the current module. Does it mess up the source checker scripts or something?

I tend to think it's best to preemptively avoid short generic names as much as possible, not because (as a developer) it's hard to look up the full path but because it should ideally be rarely if ever necessary to do so e.g. to know which "utils.h" are we talking about. I can imagine tooling issues too (like filename:LINE reporting w/o full path).

The Google C++ Style Guide recommends the same.

Also available in: Atom PDF