type: change ResourceDescriptor and Link interfaces - #403
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @dodok8, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request updates core data structures to improve compatibility with the OStatus 1.0 Draft 2 specification. It primarily involves refining how links are represented within resource descriptors, enabling support for specific OStatus subscription mechanisms and increasing the flexibility of link definitions.
Highlights
- New Interface for OStatus Subscription: Introduced the OStatusSubscribeLink interface to specifically handle subscription links as defined in OStatus 1.0 Draft 2, including a template property for URI construction.
- Optional Link Href: The href property in the Link interface has been made optional, allowing for more flexible link definitions where a direct URI might not always be present.
- Expanded Resource Descriptor Links: The links property within the ResourceDescriptor interface now supports an array that can contain both standard Link objects and the newly added OStatusSubscribeLink objects, enhancing compatibility with OStatus specifications.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
github-actions
Bot
added
activitypub/interop
There was a problem hiding this comment.
Code Review
This pull request updates the WebFinger JRD interfaces to support OStatus 1.0 Draft 2. The changes include making the href property on Link optional, adding a new OStatusSubscribeLink interface for subscription links, and updating the ResourceDescriptor to allow an array of either link type. My review has identified a few areas for improvement. Firstly, making href optional is a breaking change that should be clearly documented in the PR description and changelog. Secondly, the new OStatusSubscribeLink interface could be more complete by including optional properties from the standard Link interface. Finally, the union type for links is not a true discriminated union, which has implications for type-safety and API ergonomics that should be considered and documented. I've left specific comments with suggestions on these points.
GitHub Actions
Failure
9d754ef to
06adbe6
Compare
06adbe6 to
d4aadc2
Compare
|
The docs for this pull request have been published: |
…sSubscribeLink and make href optional
1f37da4 to
713ff70
Compare
| /** | ||
| * References a link. See also | ||
| * [OStatus 1.0 Draft 2](https://www.w3.org/community/ostatus/wiki/images/9/93/OStatus_1.0_Draft_2.pdf) | ||
| */ | ||
| export interface OStatusSubscribeLink { | ||
| rel: "http://ostatus.org/schema/1.0/subscribe"; | ||
| /** | ||
| * A URI template (RFC 6570) that can be used to construct URIs by | ||
| * substituting variables. Used primarily for subscription endpoints | ||
| * where parameters like account URIs need to be dynamically inserted. | ||
| */ | ||
| template: string; | ||
| } |
There was a problem hiding this comment.
Instead of declaring a separate OStatusSubscribeLink type, how about adding a template field to the existing Link type? I think the template field might be useful even when the rel field is not "http://ostatus.org/schema/1.0/subscribe".
There was a problem hiding this comment.
This way seems right; however, this PR and branch are mainly about separating types, so I'll close this PR and make a new PR after adding the template field and fixing the following problems.
|
The latest push to this pull request has been published to JSR and npm as a pre-release:
|
2 participants
Summary
Change ResourceDescriptor and Link interfaces to support OStatus 1.0 Draft 2
Related Issue
Linktype andResourceDescriptor#402rel= 'http://ostatus.org/schema/1.0/subscribe'andtemplate#119Changes
OStatusSubscribeLinkhreffield inLinkOptionallinksfield ofResourceDescriptortoArray<Link | OStatusSubscribeLink>Benefits
Describe the advantages or improvements brought by these changes.
Explain how these changes affect the project, users, or performance.
Checklist
deno task test-allon your machine?Additional Notes
It's a breaking change about
ctx.lookupWebFinger().