Skip to content
This repository has been archived by the owner on Sep 19, 2021. It is now read-only.

[RFC] Install only necessary dependencies for each build type. #16

Merged
merged 1 commit into from
Sep 9, 2014

Conversation

fwalch
Copy link
Member

@fwalch fwalch commented Sep 9, 2014

Don't install unnecessary dependencies (e.g. jq, but not clang or doxygen for vimpatch report) to clean up the scripts and make builds a tiny bit faster.

@@ -87,10 +84,12 @@ get_code_quality_color() {
}

(
install_clang
setup_neovim_deps
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of putting a check in each install_/setup_ function, how about wrapping the "setup steps" at this level?

function ci_install() {
  [[ "${CI}" == "true" ]] && return 0 || echo "Local build, not installing dependencies." ; return 1;
}

ci_install && (
  install_clang
  setup_neovim_deps
)

This ci_install (or whatever) function could be put in documentation.sh or common.sh.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks! Added a is_ci_build? function.

@fwalch
Copy link
Member Author

fwalch commented Sep 9, 2014

Added two small changes in commit_doc. Output is in fwalch/doc, by the way. A build is currently running.

@fwalch fwalch force-pushed the split-dependencies branch 5 times, most recently from e9df721 to e499873 Compare September 9, 2014 18:37
@justinmk
Copy link
Member

justinmk commented Sep 9, 2014

Nice, let me know when RDY.

@fwalch fwalch mentioned this pull request Sep 9, 2014
26 tasks
@fwalch
Copy link
Member Author

fwalch commented Sep 9, 2014

Using ( for enclosing dependency installation didn't work, because exported environment variables were not available later on and dependencies were built instead of using the ones from neovim/deps.
Also changed find_all_bugs_number to use its parameter.

The build passed; I think this is RDY now.

@justinmk
Copy link
Member

justinmk commented Sep 9, 2014

Good catch!

justinmk added a commit that referenced this pull request Sep 9, 2014
Install only necessary dependencies for each build type.
@justinmk justinmk merged commit f682d56 into neovim:master Sep 9, 2014
@fwalch fwalch deleted the split-dependencies branch September 10, 2014 08:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants