Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

added http interface to hc run #846

Merged
merged 12 commits into from
Jan 15, 2019
Merged

added http interface to hc run #846

merged 12 commits into from
Jan 15, 2019

Conversation

zippy
Copy link
Member

@zippy zippy commented Jan 10, 2019

this just adds an env var so people can use the http interface with hc run

@zippy zippy added the review label Jan 10, 2019
@zippy zippy changed the title added http interface to hc run WIP: added http interface to hc run Jan 10, 2019
@zippy zippy changed the title WIP: added http interface to hc run added http interface to hc run Jan 10, 2019
Copy link
Collaborator

@Connoropolous Connoropolous left a comment

Choose a reason for hiding this comment

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

@zippy I notice you've been inclined to start adding environment variables recently...
can you say what motivates that? I think we should start out with a really solid practice documentation approach for those, in a centralized place as well as in context for its use, since I felt that was a very obscure and hard to work with aspect of holochain-proto.

@zippy
Copy link
Member Author

zippy commented Jan 11, 2019

This is a good point about documentation. Motivation is that I find them a very clean easy and expected way to get data into commands that lends itself to temporary settings via export and so on.

Copy link
Collaborator

@Connoropolous Connoropolous left a comment

Choose a reason for hiding this comment

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

no tests for this worries me

@zippy
Copy link
Member Author

zippy commented Jan 12, 2019

ok @Connoropolous , I got all the tests refactored and working, locally at least, we'll see what happens with CI. But I will disable them again before merge because they still do take way too long.

@Connoropolous
Copy link
Collaborator

@zippy is this for after the release?

@zippy
Copy link
Member Author

zippy commented Jan 13, 2019

Yep not part of 0.0.3. I just pushed turning off that run test, because it takes too long for travis. You can turn it on and test locally if you like. Otherwise please re-review so we can merge using http in hc.

Copy link
Collaborator

@Connoropolous Connoropolous left a comment

Choose a reason for hiding this comment

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

Ok, well I'm still seeing issues in this area of the code, but they aren't related to this PR per say.

If we could create some followups because there's clearly something off if we are writing code that we can't test, which is true of the scaffold stuff. The essential challenge is that to consistently test it we need the scaffold values to be dynamic, but during releases, we want the values to be fixed. That should prbly be the basis of the followup.

@zippy zippy merged commit e18fe5b into develop Jan 15, 2019
@zippy zippy removed the review label Jan 15, 2019
@zippy zippy deleted the http-for-hc branch July 4, 2019 19:58
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.

3 participants