FAIRDataPipeline / cppDataPipeline

C++ Implementation of the FAIR Data Pipeline API
http://www.fairdatapipeline.org/cppDataPipeline
Other
0 stars 2 forks source link

[Discussion] Code formatting standard #11

Open RyanJField opened 2 years ago

RyanJField commented 2 years ago

Decide on a code formatting standard: Spacing - Spaces vs Tabs Function / Class / Namespace Naming

Any other suggestions for standardisation?

dezed-ukaea commented 2 years ago

I suggest clang-format for prettifying alignment, brace position etc. I don't have a view reagrding naming conventions as long as they are descriptive.

richardreeve commented 2 years ago

I know you've called the top-level namespace FDP and the library libfdpapi, but for consistency with the other code I was wondering whether it should be libfairdatapipeline, and maybe even the namespace fairdatapipeline. You can always use a namespace alias to get it back to FDP for typing convenience if you want to... at the moment we have:

Language Repo (Package) Name Namespace
R rDataPipeline rDataPipeline
Julia DataPipeline.jl (DataPipeline) DataPipeline
Python pyDataPipeline (data-pipeline-api) data_pipeline_api
Java javaDataPipeline (org.fairdatapipeline) org.fairdatapipeline
C++ cppDataPipeline (libfdpapi) FDP

Python and Java are a little bit different (Java has to be, because you have to own the domain), but C++ is the only one using FDP... that said, I don't feel that strongly about it!

RyanJField commented 2 years ago

This is also covered by issue #12 as SonarCloud doesn’t like all uppercase namespaces. I am happy with any namespace; FDP and libfdpapi was already in place when I got the code so I’m not attached to either.

richardreeve commented 2 years ago

Cool. This is only the second time I've looked at the C++ code - and the first was two weeks ago - so I didn't realise that those were old names. It makes sense though. We had previously been planning on using FDP, but there was a campaign not to (= 2 people said they hated it!), so we switched to the full name instead.

dezed-ukaea commented 2 years ago

Renaming the namespace (FDP -> ) will change the public interface. Is this acceptable? It imposes a code change on clients. Yes it can be trivially fixed with an alias but breaking public interfaces is something I have learnt is viewed dimly. If acceptable I will rename FDP -> fdp to remove a code smell until a final namespace is determined.

RyanJField commented 2 years ago

It would be better to change to the final namespace probably: FairDataPipeline with an alias to FDP. I understand breaking the public interface shouldn't be taken lightly however as we are still in an initial development version (< 1.0.0) it is better for this change to happen now.

dezed-ukaea commented 2 years ago

The client will have to do the aliasing. Aliasing to FDP in a header will make the namespace change pointless (nobody would use it) and risk clashing with external namespaces.

RyanJField commented 2 years ago

Understood; I was thinking adding the alias to the cppSimpleModel to save refactoring the code.