OPM/opm-common

Split of sources and headers

akva2 opened this issue · 10 comments

akva2 commented

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.

I vote for removing the split in opm-common.

bska commented

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
akva2 commented

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 commented

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)

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.

akva2 commented

@atgeirr suggests

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

@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?

akva2 commented

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

bska commented

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

this can be closed.