Skip to content

improve 'term definition' definition #638

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

improve 'term definition' definition #638

wants to merge 5 commits into from

Conversation

pchampin
Copy link
Contributor

@pchampin pchampin commented Feb 27, 2025

I'm not entirely satisfied with the addition in common/terms.html, as it seems too detailed for the terminology section. But...

...as pointed out in the comment of #630,
the API spec references to this definition in multiple places, and many of those links are actually for instances of the internal representation, not for instances of the strings or maps that represent term definition in JSON.

So the alternative would be to introduce a new name for those "internal term definition" and patch all the algorithms... which seems laborious and error-prone.

@gkellogg I don't remember what's the process for synchronizing the files in common with other specs.


Preview | Diff

I'm not entirely satisfied with the addition in common/terms.html,
as it seems too detailed for the terminology section. But...

...as pointed out in the comment of #630,
the API spec references to this definition in multiple places,
and many of those links are actually for instances of the internal representation,
*not* for instances of the strings or maps that represent term definition in JSON.

So the alternative would be to introduce a new name for those "internal term definition"
and patch all the algorithms... which seems laborious and error-prone.

@gkellogg I don't remember what's the process for synchronizing the files in common with other specs.
@gkellogg
Copy link
Member

@gkellogg I don't remember what's the process for synchronizing the files in common with other specs

Just needs to be copied. The GH Action checks out common and compares the results.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
@pchampin
Copy link
Contributor Author

pchampin commented Feb 28, 2025

This one probably also needs to be formatted as a candidate amendment...
And a corresponding one will be needed in json-ld-syntax.

I didn't follow the pattern 'change_N' for the id,
because this change will also be present in json-ld-syntax (in comment/terms),
and so will probably require a candidate addition in that spec too (with a different index).
Hence the more robust id used here.
@pchampin pchampin added the class-2 Class-2 change label Feb 28, 2025
Comment on lines +386 to +387
<ins cite="#change_api_638"><br/>
For <a data-cite="JSON-LD11-API#context-processing-algorithms">context processing</a>, <a>term definition</a> values are converted internally to a dedicated data structure that is easier to process.</ins>
Copy link
Member

Choose a reason for hiding this comment

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

The problem with this is that that will show up in syntax and framing, as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's why I named it change_api_638 insteal of change_8. That way, we can add a change marker in the other specs without an ID clash.

Copy link
Member

Choose a reason for hiding this comment

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

We'll need issues for those other specs to have their own version of this update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I created w3c/json-ld-syntax#461 to track this in Syntax, so I think we can probably merge this now.

@gkellogg gkellogg moved this to Discuss-Call in JSON-LD Management Mar 10, 2025
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@w3cbot
Copy link

w3cbot commented Jun 4, 2025

This was discussed during the #json-ld meeting on 04 June 2025.

View the transcript

w3c/json-ld-api#638

<gb> Pull Request 638 address #630 (by pchampin) [needs discussion] [class-2]

pchampin: This is about inverse context creation.
… I thought it was ambiguous in the definition of the active context.
… I tried to improve it without restructuring the document, which meant changing something in the terminology section.
… The end result is not 100% satisfying, as change to terminology is big.
… It's also inherited in other documents, so we need to mark changes in other documents, even though it's only in JSON-LD API.

bigbluehat: thoughts? especially on the size of the change?

bigbluehat: Looks like some validation errors.

gkellogg: I think it's good to go
… and I plan to make similar ones in the other specs
… so once we get the validation error fixed, we can merge this and make the others

bigbluehat: do we need to synchronize those changes across the specs?

gkellogg: the json-ld-commons repository is where the term changes would happen
… and then that would be copied back out to the individual spec repos
… and if we don't, then the github action will complain
… pchampin can you look at the markup issues?

pchampin: yep


@pchampin
Copy link
Contributor Author

pchampin commented Jun 4, 2025

@gkellogg I actually don't see any markup issue there; the only reported issue is the inconsistency of the common files between the different repos, which is to be expected.

@pchampin pchampin changed the title address #630 improve 'term definition' definition Jun 4, 2025
@gkellogg
Copy link
Member

gkellogg commented Jun 4, 2025

@gkellogg I actually don't see any markup issue there; the only reported issue is the inconsistency of the common files between the different repos, which is to be expected.

Great. The thing to do is to update json-ld-common with the change to the common files. After that is merged, this should build cleanly. We'll need to do similar updates to common files in other repos, and possibly rebase some open PRs.

@gkellogg gkellogg moved this from Discuss-Call to PRs in JSON-LD Management Jun 4, 2025
gkellogg added a commit to w3c/json-ld-wg that referenced this pull request Jun 5, 2025
gkellogg added a commit to w3c/json-ld-syntax that referenced this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class-2 Class-2 change
Projects
Development

Successfully merging this pull request may close these issues.

5 participants