Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace bash script with Cake.Docker task #2839

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

HebaruSan
Copy link
Member

Motivation

#2838 is setting up build triggers to rebuild an inflator container for #2789. This pull request targets that PR's branch with a suggestion for changing it.

Currently it:

  • Relies on a bash script
  • Uses the _build/repack/Release folder as its working folder
  • Hard-codes paths that are defined as variables in other places, meaning extra maintenance if those ever change

Changes

Now the bash script is replaced with a new task in the build.cake file, which:

  • Uses the standard variables to get the path to netkan.exe and _build
  • Works out of _build/docker/inflator
  • Might work on systems without bash, depending on how portable Cake.Docker is
  • Otherwise does the same things the bash script did

The Travis config is updated to rebuild the container with ./build docker-inflator.

This way the new docker logic will fit into the existing build system.

@HebaruSan HebaruSan added Pull request Build Issues affecting the build system Netkan Issues affecting the netkan data Cake Issues affecting Cake labels Aug 7, 2019
@techman83
Copy link
Member

This is great! Thanks for bringing your 🍰 expertise!

@techman83 techman83 merged commit 5046c77 into KSP-CKAN:netkan_docker Aug 8, 2019
@HebaruSan HebaruSan deleted the feature/cake-docker branch August 8, 2019 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Issues affecting the build system Cake Issues affecting Cake Netkan Issues affecting the netkan data Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants