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

Jump to bottom

Updates for status in app info and replaces InstalledApp with InstalledAppInfo #66

Merged
merged 12 commits into from May 4, 2021
Merged

Updates for status in app info and replaces InstalledApp with InstalledAppInfo #66

merged 12 commits into from May 4, 2021

Conversation

zippy
Copy link
Member

@zippy zippy commented Apr 23, 2021

Updates for holochain version that:

  1. adds status to InstalledAppInfo
  2. returns InstalledAppInfo on install instead of InstalledApp which is supposed to be an internal Holochain structure

@zippy zippy requested review from maackle and JettTech April 23, 2021 22:19
JettTech
JettTech reviewed Apr 26, 2021
View reviewed changes
Copy link
Contributor

@JettTech JettTech left a comment •
edited

Choose a reason for hiding this comment

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

Just a few edits to run CI properly as well as update to latest commit hash and response structure. (Currently CI is showing a false positive: the conductor is unable to spin up and therefore no tests are run, which render a result of "0 of 0 passing".)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
install-holochain.sh Outdated Show resolved Hide resolved
test/e2e/index.ts Outdated Show resolved Hide resolved
test/e2e/index.ts Outdated Show resolved Hide resolved
test/e2e/index.ts Outdated Show resolved Hide resolved
@zippy @JettTech
Apply suggestions from code review
34acd25 
Co-authored-by: Lisa Jetton <ldjetton21@gmail.com>
@zippy zippy marked this pull request as ready for review April 27, 2021 14:19
JettTech
JettTech approved these changes Apr 27, 2021
View reviewed changes
Copy link
Contributor

@JettTech JettTech left a comment •
edited

Choose a reason for hiding this comment

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

Nice addition to the test. 👍
Looks like the tests are not running on CI still. I approve once that's fixed

@zippy zippy merged commit 9ce90c8 into develop May 4, 2021
@timotree3 timotree3 deleted the app-info-reason branch October 29, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Connoropolous Connoropolous Connoropolous left review comments

@JettTech JettTech JettTech approved these changes

@maackle maackle Awaiting requested review from maackle

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@zippy @Connoropolous @JettTech @alastairong @zo-el