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

[PWA URL Handling] Update URL handling explainer w/ feedback suggestions. #316

Merged
merged 8 commits into from
May 28, 2020

Conversation

LuHuangMSFT
Copy link
Contributor

@LuHuangMSFT LuHuangMSFT commented May 12, 2020

  • Changes all usage of "URI" to "URL"
  • Changes "excludePaths" to "exclude_paths" to follow JSON casing rules
  • Changes "comment" to "description". This is an optional but recommended field that aids readability of the pwa-site-association file instead of a normal JSON comment
  • Add "allow_all_associations" optional field to the top level of the pwa-site-associations JSON object. This allow site owners to allow handling from any app.

Resolves #295, resolves #296, resolves #297.

@domenic
Copy link

domenic commented May 12, 2020

Nice!

I don't think this is a proper resolution of #297. In particular the description field has no processing model, so it should not exist in a specification.

@LuHuangMSFT
Copy link
Contributor Author

Nice!

I don't think this is a proper resolution of #297. In particular the description field has no processing model, so it should not exist in a specification.

Do you mean that it is not processed or used? I don't feel very strongly about this. I can remove description if it is a clear anti-pattern.

@LuHuangMSFT
Copy link
Contributor Author

description can be an implementation detail instead of being in the explainer. Does that work?

@domenic
Copy link

domenic commented May 12, 2020

How does your implementation intend to use description?

@LuHuangMSFT
Copy link
Contributor Author

How does your implementation intend to use description?

It is only for human readers, similar to comment in apple-app-site-association.

@domenic
Copy link

domenic commented May 12, 2020

If it is not going to impact implementations, then it is best removed from the spec.

@LuHuangMSFT
Copy link
Contributor Author

Removed description.

Copy link
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

I'm concerned about the addition of allow_all_associations, and I added an additional comment to #301 to try and discuss that aspect further. You may want to consider removing that specific change from this PR and then merging it out since the rest of it looks good, and then tackle #301 in a separate PR once the discussion there resolves itself.

@LuHuangMSFT
Copy link
Contributor Author

Removed "allow_all_associations" field from this PR to allow more discussion on #301.

@LuHuangMSFT
Copy link
Contributor Author

As the current changes in this PR are almost entirely URI->URL and JSON formatting changes, and because we would like to migrate this explainer to a WICG repo as soon as possible, I will complete this PR and close the 3 related issues. If the issues should not be resolved, please reopen them in the WICG repo. Thanks.

@LuHuangMSFT LuHuangMSFT merged commit eb0d48f into master May 28, 2020
@LuHuangMSFT LuHuangMSFT deleted the users/luhua/feedback_update branch May 28, 2020 05:08
@LuHuangMSFT LuHuangMSFT changed the title Update URL handling explainer w/ feedback suggestions. [PWA URL Handling] Update URL handling explainer w/ feedback suggestions. Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants