vector-of-bool / pitchfork

Pitchfork is a Set of C++ Project Conventions
MIT License
1.12k stars 56 forks source link

Feedback on cxx-pflR1 (2018-11-14) #11

Open FlorianWolters opened 6 years ago

FlorianWolters commented 6 years ago

Hello,

I'm pretty late to the party and stumbled across Pitchfork two days ago. I'm very interested in the future of the C++ ecosystem (build/tool integration as well as modernization/modern usage of the C++ language itself). I do think a common filesystem for C++ (and C) source code would be great. So, thanks for you effort!

I've been using a (self-created) CMake-based Convention over Configuration (CoC) for some time now and I think I'm pretty good and confident using CMake and a component-centric approach using a small-to-medium scale code base. I.e. I know what works in general and what not. My framework is almost zero-conf, except dependency management: dependencies are listed in CMakeLists.txt, but find_package calls are propagated transitively to downstream projects by the system. Using a package manager is not possible yet.

Suprisingly (or not) I'm already using the same concepts (library-centric, one library per project, distinction between "app" and "lib" projects, etc.) and file system layout as defined in the PFL (with some minor modifications).

I'm using this issue to provide some feedback to you. Please tell me if I can help with anything else (though my time is pretty limited).

Again: I think the PFL is an awesome idea. The C++ ecosystem lacks so much compared to other (mostly managed) languages.

A common file system layout and a common/spread CoC build system (potentially using CMake) enforcing that layout would be super nice, imo.

If that CoC build system would (as opt-ins) support most of the open-source software C++ development tools (e.g. Cppcheck, Doxygen, clang-tidy), beginners could have a fun-time starting with Modern C++.

As it is now, I cannot recommend a (sane) beginner to start programming using C++, since dealing with the ecosystem is a pain in the a**.

Resources

Though, I did not completely read all of the following resources, I think they should be taken into account and/or referenced.

In addition, they may provide information for further improvements. I may add additional items in the future here.

My current filesystem suggestion

Note that the following listing does not include "resource" files yet, since it is pretty late here already.

But it includes everything related to source code files (except example).

  │   .clang-format
  │   .clang-tidy
  │   .cppcheck
  │   .gitignore
  │   CHANGELOG.md
  │   CONTRIBUTING.md
  │   LICENSE
  │   README.md
  │
  ├───app
  │   │   hello_world.cpp
  │   │   tower_of_hanoi.cpp
  │   │
  │   └───cli_greeter
  │       │   cli_greeter.cpp
  │       │
  │       ├───include
  │       │       greeter.h
  │       │
  │       └───src
  │               greeter.cpp
  │
  ├───include
  │   └───fw
  │           fw.h
  │           my_class.h
  │
  ├───src
  │   └───fw
  │           fw.cpp
  │           my_class.cpp
  │           my_class_test.cpp
  │
  └───test
      ├───app
      │   │   simple_integration_test.cpp
      │   │
      │   └───complex_integration_test
      │       │   complex_integration_test.cpp
      │       │
      │       ├───include
      │       └───src
      ├───include
      │   └───fw
      │       └───testing
      │               my_test_util_class.h
      │               testing.h
      │
      └───src
          └───fw
              └───testing
                      my_test_util_class.cpp

Regards

vector-of-bool commented 6 years ago

Thanks for the lengthy write-up! Sorry to take a few days to get back to you, though.

Where do we store Interface Definition Language

You aren't the first person to ask. I'm open to suggestions. So far I've just said "Eh... put it in src/" but I think a more thorough answer is warranted.

I recommend using singular instead of plural for all directory names

I keep flip-flopping on the pluralization of directory names. I'll need to consolidate on one and commit to it.

What is the reason for using the .hpp file extension for header files?

I don't mandate a specific extension, but I do recommend one. I chose .hpp to specifically call out the distinction between .h files. There could be huge debate about this aspect. Also, this project was partially made in response to the Core Guidelines regarding aspects like file layout. The "Just be consistent" methodology keeps people in their own "islands of consistency".

I don't think that the build directory should be specified by a C++ filesystem layout convention.

I don't either. build/ and _build/ are reserved, but not required. The concept of an "install" directory is not mentioned because installing inside of the project directory is not common, and should be done inside of the build directory if not to some external path.

As for the distinction between executable sources and library sources, there is still a heated debate on the topic.

I suggest removing the sentence "Other files should not appear in the root of the project."

I'll add support for the common CHANGELOG and CONTRIBUTING files, but I'd still rather encourage most user-readable content to go in the docs/ directory.

tests/: In my opinion the layout of that directory has to be specified by the PFL. Otherwise we have to consider that .cpp test source files are stored in multiple arbitrary subdirectories below that directory.

If there's one thing less consistent that building a project, that would be testing a project. I'm not sure what the best test layout would look like. For some projects, individual .cpp files are generated straight to executables, and a whole large structure is not helpful. For those with large test infrastructure, more provisions are warranted. This varies wildly between build systems and testing libraries, harnesses, and methodologies. This may be the same basis for the problem of finding a good layout for distinguishing between executable sources and library sources.

Where is the .test convention derived from?

John Lakos originally recommends using something.t.cpp. I'm not a fan of the needless brevity, so I adapted it to something.test.cpp. This particular point was actually hugely contentious when I posted it on my blog, and the "requirement" has been lowered to "recommendation."

A common file system layout and a common/spread CoC build system (potentially using CMake) enforcing that layout would be super nice, imo.

You may be interested in Evoke which is a convention-oriented build system. I've also written a pf_auto() function that automatically sets up a CMake-based build using the directory structure. You can see it used in the CMakeLists.txt for this project.

Thanks for all the feedback!

Mike-Devel commented 6 years ago

One thing I'd like to say about tests: It imho doesn't make sense to have the test files directly next to the source files, because they have completely different prupose, requirements and are used in different ways from the "regular" source files. Just as a few examples of what I mean:

So, I aknowledge, that there is a strong argument for putting unit-test files next to the files they test due to the strong coupling between them. However, my understanding is that one of the main purposes of this proposal is to make building, consumption, deploying and generally tooling of c++ projects easier and with respect to all those points, a unit-test file has to be handled differently and has different requirements than "normal" cpp files and as such I think they should be placed into a different directory.

FrankHB commented 3 years ago

Thanks for the lengthy write-up! Sorry to take a few days to get back to you, though.

Where do we store Interface Definition Language

You aren't the first person to ask. I'm open to suggestions. So far I've just said "Eh... put it in src/" but I think a more thorough answer is warranted.

Seems some updates are needed in the "Open Questions" section of the document.

But the specific answer here might be simple: put them either in include/ or src/.

A native C++ project can certainly have source code files not conforming to the C++ standard. Consider C and assembly sources used to be included by the preprocessor (which is not necessarily a part of the same implementation of C++ being used to build the C++ source here). IDL does not differ in this sense.

Some loosely relavant: putting .bs into data/ seems wrong to me. This is also a form of human-readable source code, even it never build to program executable or library blobs. Or put it in docs/?

I recommend using singular instead of plural for all directory names

I keep flip-flopping on the pluralization of directory names. I'll need to consolidate on one and commit to it.

Or just provide the normative allowence of synonyms, with some predefined lists of alternative.

This is needed for some extra consistency. Consider include + src: would include + source or inc + src be strictly more correct? But anyway, include + src is already widely used for historical reasons, even in some standards. I don't think sticking to one choice is good enough. (There might be the amibguity of "source" constrast to "binary package" in the sense of the package management, though. Such use can be also used in a layout convention like this.)

It also allows to resolve t vs test by users themselves.