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

Result view updated to make intent from introductory video work #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

a-stefanski
Copy link

On bixbydevelopers.com we have introductory video
(https://bixbydevelopers.com/dev/docs/sample-capsules/videos/fundamentals)
where sample intent is presented (see https://youtu.be/EYp_AhgNQWA?t=1415).
Since intent returns 3 items and result-view is updated to handle only 1 item developer which follows video with cloned code gets empty view. This can make confusion.

I suggest to keep this example working by leaving code snippet to handle more than one item in result view.

Not working intent from itroductory video:
intent {
goal {SpaceResort}
value {Planet (Jupiter)}
value {SearchCriteria (refueling services)}
}

@a-stefanski a-stefanski changed the title Result view updated to make work intent from introductory video Result view updated to make intent from introductory video work Jul 16, 2019
@sarahmolinerroy
Copy link
Contributor

In order to add support for property projections, we have since refactored this capsule so the outer goal is SpaceResort#all instead of SpaceResort. So the intent you shared is not modeled in the capsule. If you use NL instead of raw intent, does it work?

@a-stefanski
Copy link
Author

I see how it works. I was suprised that going through video called "Bixby Fundamentals Learn step-by-step all aspects of developing capsules." using reference source code I got broken result.
I am afraid that this can be discouraging experience for some. Shouldn't we keep consistency between introductory videos and reference code for them?

@sarahmolinerroy
Copy link
Contributor

@jatinbviv @siamakh What do you think? Should we revert the property projections to simplify this capsule again. Maybe only host them on a different branch instead of master?

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.

2 participants