Review of VOTCA actions

[JoshuaSBrown]

Review and create documentation for clarifying build system.

[JoshuaSBrown]

The setup.sh script is missing a description at the top illustrating how it should be called, what arguments should be passed to it.

[JoshuaSBrown]

Can you explain what this is doing?

if [[ ${INPUT_BRANCH} ]]; then # user overwrite
  branch="${INPUT_BRANCH}"
elif [[ ${GITHUB_REF} = refs/pull/*/merge ]]; then # pull request
  branch="${GITHUB_BASE_REF}"
elif [[ ${GITHUB_REF} = refs/heads/* ]]; then # branch, e.g. stable
  branch=${GITHUB_REF#refs/heads/}
elif [[ ${GITHUB_REF} = refs/tags/* ]]; then # tag or release
  branch=${GITHUB_REF#refs/tags/}
else
  die "Handling on GITHUB_REF=${GITHUB_REF} not implemented"
fi

You are checking if the branch is a valid branch, and also allowing a user to override what branch to use if it is passed in as an option to setup.sh?

[JoshuaSBrown]

These die commands could probably be a little more helpful

[[ ${module} ]] || die "Could not fetch module"

Could not determine module from CMakeLists.txt file, no project(votca-....) found

die "Unknown INPUT_TOOLCHAIN"

die "Unknown INPUT_TOOLCHAIN ${INPUT_TOOLCHAIN}"

[JoshuaSBrown]

What is the purpose of this:

if [[ ${INPUT_MODULE} = true ]]; then
  cmake_args+=( -DMODULE_BUILD=ON -DCMAKE_INSTALL_PREFIX=$HOME/votca.install )
else
  cmake_args+=( -DCMAKE_INSTALL_PREFIX=/usr )
fi

[junghans]

Can you just a make a PR?

[junghans]

The setup.sh script is missing a description at the top illustrating how it should be called, what arguments should be passed to it.

https://github.com/votca/actions/blob/master/setup/action.yml#L3-L41

[junghans]

Can you explain what this is doing?

if [[ ${INPUT_BRANCH} ]]; then # user overwrite
  branch="${INPUT_BRANCH}"
elif [[ ${GITHUB_REF} = refs/pull/*/merge ]]; then # pull request
  branch="${GITHUB_BASE_REF}"
elif [[ ${GITHUB_REF} = refs/heads/* ]]; then # branch, e.g. stable
  branch=${GITHUB_REF#refs/heads/}
elif [[ ${GITHUB_REF} = refs/tags/* ]]; then # tag or release
  branch=${GITHUB_REF#refs/tags/}
else
  die "Handling on GITHUB_REF=${GITHUB_REF} not implemented"
fi

You are checking if the branch is a valid branch, and also allowing a user to override what branch to use if it is passed in as an option to setup.sh?

We are figuring out what base branch to use for votca/votca and other checks, e.g. some CMake options only work on the stable branch.

[junghans]

What is the purpose of this:

if [[ ${INPUT_MODULE} = true ]]; then
  cmake_args+=( -DMODULE_BUILD=ON -DCMAKE_INSTALL_PREFIX=$HOME/votca.install )
else
  cmake_args+=( -DCMAKE_INSTALL_PREFIX=/usr )
fi

If we are doing a module build, install stuff in a different prefix. The module builds installs stuff in the build step and as we don't run the build with sudo it needs to be in $HOME.

[junghans]

Can you just a make a PR?

For the trivial changes.

[JensWehner]

Is this still relevant? @junghans

[junghans]

Are you guys happy with the documentation now?