Skip to content
This repository was archived by the owner on Apr 24, 2023. It is now read-only.

Adds travis build with pools turned off#885

Merged
pschorf merged 4 commits intotwosigma:masterfrom
dposada:no-pools-build
Jun 19, 2018
Merged

Adds travis build with pools turned off#885
pschorf merged 4 commits intotwosigma:masterfrom
dposada:no-pools-build

Conversation

@dposada
Copy link
Contributor

@dposada dposada commented Jun 14, 2018

Changes proposed in this PR

  • adding --pools={on/off} to run_integration.sh
  • adding a build to the travis job that runs with --pools=off

Why are we making these changes?

We want to ensure that changes we make for pools don't break environments that don't have pools.

@dposada dposada removed the wip label Jun 14, 2018
@dposada dposada requested a review from pschorf June 14, 2018 17:59
@dposada
Copy link
Contributor Author

dposada commented Jun 15, 2018

@pschorf Thoughts on this?

(let [pools (all-pools db)
default-pool-name (config/default-pool)]
(log/info "Pools in the database:" pools ", default pool:" default-pool-name)
(if default-pool-name
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be better in the config section, so everyone else doesn't have to worry about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By "this", do you meany the blank? check?

Copy link
Contributor

@pschorf pschorf Jun 15, 2018

Choose a reason for hiding this comment

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

Right, either replacing blank with nil or failing to load the config.

Sorry for the lack of clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dposada
Copy link
Contributor Author

dposada commented Jun 18, 2018

@pschorf This is ready for another pass

@dposada
Copy link
Contributor Author

dposada commented Jun 19, 2018

@pschorf Any other feedback on this?

@pschorf pschorf merged commit 3c9285b into twosigma:master Jun 19, 2018
@dposada dposada deleted the no-pools-build branch June 20, 2018 14:22
dposada added a commit to dposada/Cook that referenced this pull request Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants