opm-common icon indicating copy to clipboard operation
opm-common copied to clipboard

Split of sources and headers

Open akva2 opened this issue 3 years ago • 8 comments

opm-common is currently the only repo where we have a src/ directory for sources (and private headers).

Should we make this consistent? Either by doing this everywhere or by removing the split in opm-common?

Personally I'm of the latter opinion.

akva2 avatar Nov 30 '22 09:11 akva2

I vote for removing the split in opm-common.

totto82 avatar Nov 30 '22 09:11 totto82

I too think the cost of the split is a bit high, but I also think we should keep in mind that there are, as you say, a few private headers in the src directory. Whether or not they should be is of course debatable, but the current split makes it easier to keep the components private.

Private Headers

  • src/opm/input/eclipse/EclipseState/Aquifer/AquiferHelpers.hpp
  • src/opm/input/eclipse/EclipseState/Grid/Operate.hpp
  • src/opm/input/eclipse/EclipseState/Grid/setKeywordBox.hpp
  • src/opm/input/eclipse/Parser/raw/RawConsts.hpp
  • src/opm/input/eclipse/Parser/raw/RawEnums.hpp
  • src/opm/input/eclipse/Parser/raw/RawKeyword.hpp
  • src/opm/input/eclipse/Parser/raw/RawRecord.hpp
  • src/opm/input/eclipse/Parser/raw/StarToken.hpp
  • src/opm/input/eclipse/Python/EmbedModule.hpp
  • src/opm/input/eclipse/Python/PyRunModule.hpp
  • src/opm/input/eclipse/Python/PythonInterp.hpp
  • src/opm/input/eclipse/Schedule/Action/ActionParser.hpp
  • src/opm/input/eclipse/Schedule/eval_uda.hpp
  • src/opm/input/eclipse/Schedule/MSW/Compsegs.hpp
  • src/opm/input/eclipse/Schedule/MSW/FromWSEG.hpp
  • src/opm/input/eclipse/Schedule/MSW/icd_convert.hpp
  • src/opm/input/eclipse/Schedule/UDQ/UDQParser.hpp
  • src/opm/input/eclipse/Schedule/Well/injection.hpp

bska avatar Nov 30 '22 10:11 bska

They definitely should stay private if they can be private. Move them to opm/private/* ? Just keep as they are (and still keep off installation list) ?

akva2 avatar Nov 30 '22 10:11 akva2

Ie

#!/bin/bash

PRIV_HEADERS=`find src/ -name "*.hpp"`
SOURCES=`find src/ -name "*.cpp"`

for HDR in $PRIV_HEADERS
do
  DDIR=`dirname $HDR | sed -e 's/src\/opm\///g'`
  DF=`echo $HDR | sed -e 's/src\/opm\///g'`
  echo "mkdir -p opm/private/$DDIR"
  echo "git mv $HDR opm/private/$DF"
done

for SRC in $SOURCES
do
  DDIR=`dirname $SRC | sed -e 's/src\///g'`
  DF=`echo $SRC | sed -e 's/src\///g'`
  echo "mkdir -p $DDIR"
  echo "git mv $SRC $DF"
done

(ignore the echos)

akva2 avatar Nov 30 '22 10:11 akva2

I vote for removing the split. I think private headers are annoying because often at some point we want to make it public, but putting them in a private/ or details/ dir is fine with me. Then it is clear it is not intended as public API, while also making it easy to change that.

atgeirr avatar Nov 30 '22 13:11 atgeirr

@atgeirr suggests

git mv src/opm/x/y/z/foo.hpp -> opm/x/y/z/details/foo.hpp

akva2 avatar Nov 30 '22 13:11 akva2

@atgeirr suggests

opm/*/details/header.hpp

I think that's fine. The only possible exception is src/opm/input/eclipse/Parser/raw/* which does not already have a corresponding public component (i.e., opm/input/eclipse/Parser/raw). In that case I think we can just use opm/input/eclipse/Parser/details instead.

That said, I think I'd like to know a little bit more about the context here. Is this intended as aa preparatory step to enabling some other work, or are you contemplating the reorganisation for some other reason?

bska avatar Dec 01 '22 11:12 bska

no technical reason, only that people (in particular @atgeirr) has expressed that they would like the unification.

akva2 avatar Dec 01 '22 12:12 akva2

@akva2 : Didn't we already do this? Is this issue still relevant?

bska avatar Aug 15 '24 11:08 bska

this can be closed.

akva2 avatar Aug 15 '24 11:08 akva2