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

AppD consolidated update PR #668

Merged
merged 62 commits into from
May 27, 2022
Merged

Conversation

kriswest
Copy link
Contributor

@kriswest kriswest commented Apr 20, 2022

Consolidated PR for AppD updates (each of which is raised as a separate PR into this branch for review by AppD discussion group members, maintainers and editors).

resolves #369
resolves #370
resolves #553
resolves #555
resolves #556
resolves #558
resolves #560
resolves #561
resolves #562
resolves #565
resolves #566
resolves #567
resolves #622

@kriswest kriswest added enhancement New feature or request app-directory labels Apr 20, 2022
@kriswest kriswest requested a review from ggeorgievx April 20, 2022 15:37
kriswest and others added 21 commits April 20, 2022 17:36
…upport localized copies of descriptive fields and of the apps themselves
Co-authored-by: Hugh Troeger <troeger.hugh@gmail.com>
560 Defining a term for a single appD record
555 556 558 re-write of the AppD overview page
@kriswest kriswest changed the title [WIP] AppD consolidated update PR AppD consolidated update PR May 16, 2022
@robmoffat
Copy link
Member

robmoffat commented May 18, 2022

Good job - read this pretty closely didn't notice any typos / errors.

I've approved but I have this one note:

I notice this page hasn't received any changes: https://deploy-preview-668--lambent-kulfi-cf51a7.netlify.app/docs/next/app-directory/discovery

I was reading this through again, and to my (untrained, noob) eye the part about "Application ID namespace syntax host resolution" really stands out as not relevant now. If we're moving to the model we talked about on Monday of "get all the appD records from all the app directories" then I don't see what use this naming scheme is.

The only use-case I can see is one app wanting to load another.. but if that was happening, I don't think the app would know anything about the app directory, would it?

@kriswest kriswest mentioned this pull request May 20, 2022
17 tasks
Copy link
Contributor

@mattjamieson mattjamieson left a comment

Choose a reason for hiding this comment

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

LGTM!

CHANGELOG.md Outdated Show resolved Hide resolved
src/app-directory/specification/appd.yaml Outdated Show resolved Hide resolved
src/app-directory/specification/appd.yaml Outdated Show resolved Hide resolved
website/static/schemas/next/app-directory.yaml Outdated Show resolved Hide resolved
website/static/schemas/next/app-directory.yaml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Hugh Troeger <htroeger@factset.com>
@kriswest kriswest requested a review from hughtroeger May 25, 2022 11:39
@kriswest
Copy link
Contributor Author

@greyseer256 changes applied

@kriswest
Copy link
Contributor Author

kriswest commented May 25, 2022

@robmoffat apologies, thought I'd replied to the below:

I notice this page hasn't received any changes: https://deploy-preview-668--lambent-kulfi-cf51a7.netlify.app/docs/next/app-directory/discovery

I was reading this through again, and to my (untrained, noob) eye the part about "Application ID namespace syntax host resolution" really stands out as not relevant now. If we're moving to the model we talked about on Monday of "get all the appD records from all the app directories" then I don't see what use this naming scheme is.

The only use-case I can see is one app wanting to load another.. but if that was happening, I don't think the app would know anything about the app directory, would it?

The discovery page got merged into the overview in the formal spec re-org and shuffled around a little so that the manual config of appD was promoted to first in the list (as the primary way appDs are discovered IRL). The group hasn't discussed discovery as yet but can look at it in future. Its worth noting that the main use-case for fully qualified appIds is making them globally unique, rather than discovery. However, I think it will remain useful to be able to track back to the appD from the fully-qualified Id - perhaps more so once Desktop Agent Bridging arrives and you start seeing comms between apps on different desktop agents, which may not know about the same appDs as each other...

@kriswest kriswest merged commit 0c5bb50 into master May 27, 2022
@kriswest kriswest deleted the Appd-consolidated-update-branch branch May 27, 2022 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment