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

Build pool fixes #136

Merged
merged 8 commits into from
Aug 31, 2018
Merged

Build pool fixes #136

merged 8 commits into from
Aug 31, 2018

Conversation

cam-parra
Copy link

The file is now not a demo tool and more of a dev tool

@cam-parra cam-parra requested a review from brentzundel August 29, 2018 16:11
@@ -56,8 +56,8 @@ ARG python3_indy_crypto_ver=0.4.1
ARG indy_crypto_ver=0.4.0
ARG indy_plenum_ver=1.6.50

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel there should be a comment here to explain that these values are dynamically supplied by the prep script.

Copy link
Contributor

Choose a reason for hiding this comment

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

by this I mean sovtoken_pkg_name and sovtokenfee_pkg_name

Copy link
Author

Choose a reason for hiding this comment

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

I removed the names since they should always be supplied by the prep script

Copy link
Contributor

Choose a reason for hiding this comment

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

exactly, which is why a comment to that effect would be really helpful, otherwise someone trying to debug might be confused.

@ryanwwest
Copy link
Contributor

This will help with debugging errors, thanks!

@brentzundel brentzundel merged commit b3f82a1 into master Aug 31, 2018
@cam-parra cam-parra deleted the build-pool-fixes branch August 31, 2018 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants