w3c / rdf-schema

https://w3c.github.io/rdf-schema/
Other
5 stars 1 forks source link

small HTML fixes #8

Closed domel closed 1 year ago

domel commented 1 year ago

This PR is no longer about erratum 31. The following is kept only to historical reasons.

see https://www.w3.org/2001/sw/wiki/RDF1.1_Errata#erratum_31 https://lists.w3.org/Archives/Public/public-rdf-comments/2021Dec/0000.html https://lists.w3.org/Archives/Public/public-rdf-comments/2021Dec/0002.html


Preview | Diff

pfps commented 1 year ago

Why is the change being made to the range instead of to the wording when the summary persuasively argues that the change should be made to the wording?

TallTed commented 1 year ago

@pfps — Could you suggest the edit you would make, as suggested revision(s) (a/k/a requested change(s)) of this PR?

pfps commented 1 year ago

There are two parts to this PR. One part is strictly editorial and could be separated out into its own PR. The other is a change to the rdfs:range of rdf:predicate from rdfs:Resource to rdfs:Property. The rest of the section on rdf:predicate reads as if the values of this property are of necessity instances of rdf:Property (not rdfs:Property as in the PR).

The question is what change is to be made, if any. Part of the contents of www.w3.org/1999/02/22-rdf-syntax-ns are also relevant to the change, namely

rdf:predicate a rdf:Property ;
    rdfs:isDefinedBy <http://www.w3.org/1999/02/22-rdf-syntax-ns#> ;
    rdfs:label "predicate" ;
    rdfs:comment "The predicate of the subject RDF statement." ;
    rdfs:domain rdf:Statement ;
    rdfs:range rdfs:Resource .

https://www.w3.org/2001/sw/wiki/RDF1.1_Errata#erratum_31 lists this as an editorial change which is not the case for the change suggested in https://lists.w3.org/Archives/Public/public-rdf-comments/2021Dec/0000.html and in this PR. A change that is more along the lines of an editorial change would be to replace states that S is an instance of rdf:Statement, that P is an instance of rdf:Property and that the predicate of S is P. with states that S is an instance of rdf:Statement and that the predicate of S is P., i.e., removing the wording implying that values of rdf:predicate are of necessity instances of rdf:Property. If this is the change that is made there probably should be an explanation something like the following:

Instances of rdfs:Statement are not triples in an RDF graph so their values for rdf:predicate do not need to be instances of rdf:Property in that graph, although in most cases they will be.

pfps commented 1 year ago

The title of this PR and one of its commits appears to be misleading. There is no longer a change to the range of rdf:predicate.

pfps commented 1 year ago

The wording, "states that S is an instance of rdf:Statement , that P is an instance of rdf:Property and that the predicate of S is P." that initially caused the bug report is not being changed by the current PR but should be.

pfps commented 1 year ago

If the range of rdf:predicate is not changed, the title of this PR should be changed.

pfps commented 1 year ago

Right now this PR only has two simple HTML changes, nothing about the range of rdf:predicate. I'm going to change the name of the PR. I suggest that it then be merged in as a single commit (using the squash option). I'll create a separate PR for the fixes for the erratum.

pfps commented 1 year ago

@TallTed I don't think that my suggestion should moot a description of how to suggest or request changes to a PR that go outside the lines showing up in the changed files page.

pfps commented 1 year ago

@pchampin Could you check to see whether your requested changes have been satisfactorily addressed?

TallTed commented 1 year ago

@pfps --

If I understand this correctly --

@TallTed I don't think that my suggestion should moot a description of how to suggest or request changes to a PR that go outside the lines showing up in the changed files page.

-- I would suggest that you create a PR against the fork behind this PR.

Assuming acceptance, your PR can be merged into that fork, and then this PR can be merged into main.

pfps commented 1 year ago

@TallTed This is something that I have never done before. I can't even pull that branch to my clone of the repository. I get fatal: Need to specify how to reconcile divergent branches.

TallTed commented 1 year ago

@pfps — Ah, you're one of those CLI git users.

As a browser-interface user, I have never cloned the repo myself (though GitHub has done so for me). I would go to https://github.com/w3c/rdf-schema/tree/predicate-range; drill to the file I want to edit (appears likely to be spec/index.html); click the pencil icon (upper-right of the source pane within the browser tab); make my desired edits to the body; scroll to the bottom and make decisions about a new fork (for a pull request) or a direct merge (for no pull request), add a short description of my edits, and the like; then click "make pull request" on the next page. Most of this is follow-your-nose kind of stuff, though it can be confusingly simple for people used to git CLI and confusingly complex for people not used to source control.

Perhaps that gets you somewhere?

pfps commented 1 year ago

@pchampin It appears that you are blocking this PR

domel commented 1 year ago

Now this PR does not change anything. Can I close it?

pfps commented 1 year ago

The PR still adds some HTML markup, so it's not doing nothing.

domel commented 1 year ago

@pfps I double check and there is no markup edits. In addition, I know my commit so that there is nothing there. 🙂 What you see is generated automatically by respec.

pfps commented 1 year ago

I click on Files changed and I see two changes.

TallTed commented 1 year ago

@pchampin — I see two HTML changes, wrapping two strings (rdf:predicate and rdf:object) in <code> tags. I think these changes are worthwhile, and could be considered "small HTML fixes". Would you please approve this PR, such that hopefully @pfps can merge it?