allow inline location string data - #131
Conversation
🦋 Changeset detectedLatest commit: 3d45228 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR extends the app.certified.location lexicon schema to support string-based location representations alongside existing URI and blob formats. It includes schema updates, comprehensive documentation, and a changeset entry documenting the minor release. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This pull request adds support for inline location string data to the app.certified.location lexicon, allowing location data to be specified as a direct string value (coordinates or GeoJSON) in addition to the existing URI and blob formats.
Changes:
- Extended location schema to support three data formats: URI references, embedded blobs, and inline strings
- Added comprehensive documentation with examples demonstrating all three location data formats
- Updated changeset to reflect the minor version bump
- Regenerated ERD diagrams to reflect the schema changes
Reviewed changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| lexicons/app/certified/location.json | Added #string ref to location union and defined new string type for inline location data |
| README.md | Added "Creating Location Records" section with examples for all three location data formats (URI, blob, string) |
| ERD.svg | Regenerated entity relationship diagram to reflect schema changes |
| ERD-with-fields.svg | Regenerated detailed ERD with field information |
| .changeset/extend-location-schema.md | Added changeset documenting the minor version change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lexicons/app/certified/location.json (1)
36-43: Update the location description to include the string format.The union now supports strings, but the description still says “URI or blob.”
✅ Suggested fix
- "description": "The location of where the work was performed as a URI or blob." + "description": "The location of where the work was performed as a URI, blob, or inline string."
🤖 Fix all issues with AI agents
In `@lexicons/app/certified/location.json`:
- Around line 65-68: Update the SCHEMAS.md entry for app.certified.location to
list all three union members (URI, blob, and string) for the location field:
find the documentation block that currently says "URI or blob" for
app.certified.location and expand it to include the "string" variant (describe
that it can be a coordinate string or small GeoJSON), ensuring the union types
match the lexicon definition (app.certified.location -> location field) and
update examples if present to show all three options.
In `@README.md`:
- Around line 109-113: The example object wrongly places the required field
createdAt under the "Optional fields" section; update the README example so that
createdAt is shown as a required field (move it out of the optional block or
adjust the comment) and ensure the example still contains name, description, and
createdAt so integrators see createdAt is mandatory (refer to the createdAt
property in the example, and the surrounding "Optional fields" label).
- Around line 149-171: The location union in
lexicons/app/certified/location.json currently references the scalar ref
"#string", which violates ATProto Lexicon (union members must be object
variants); fix the schema by either removing the "#string" member from the union
(leaving only the existing object variants like uri and smallBlob) or replace
the "#string" ref with a proper object wrapper definition (e.g., a new record
type such as "string-wrapper" that contains a single "string" field) and update
the union to reference that object; ensure the union variant names and $type
discriminator usage in the "location" union remain consistent with other defs
(compare org.hypercerts.defs uri/smallBlob patterns) and adjust README examples
only if the schema change alters the expected payload shape.
Without this patch, the app.certified.location schema only supported location data as external URI references or embedded blobs, requiring users to store simple coordinate strings or small GeoJSON data externally or use the blob format. This is a problem because many use cases involve simple coordinate pairs or small GeoJSON strings that are more conveniently stored inline rather than as external references or binary blobs. This patch solves the problem by adding a new 'string' definition to the app.certified.location lexicon, allowing the location field to accept inline coordinate strings or GeoJSON. The README now includes comprehensive examples showing all three formats (URI, blob, and inline string) with practical use cases for each. Changes: - Add 'string' def to app.certified.location lexicon - Update location union to include '#string' reference - Document all three location data formats in README - Add code examples for coordinates and GeoJSON strings - Create changeset for minor version bump Co-authored-by: Claude Code <noreply@anthropic.com>
4b4c517 to
0d9ab59
Compare
6511f0b to
3d45228
Compare
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.