Closed (fixed)
Project:
Drupal core
Version:
8.3.x-dev
Component:
hal.module
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
9 Oct 2013 at 13:36 UTC
Updated:
25 Apr 2017 at 19:15 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
clemens.tolboomI'm not sure I understand this issue. In #2300979: Hal _links are not conform hal rfc I expected these IANA link relations for HAL.
Is this issue only about code cleanup or also for output JSON and HAL+JSON ?
Comment #2
wim leersI think this is saying
\Drupal\rest\LinkManager\RelationLinkManagerInterface::getRelationUri()is technically incorrect, it should actually have been\Drupal\rest\LinkManager\RelationLinkManagerInterface::getRelation(), because it doesn't have to be a URI, it can also be something likeedit-formorcanonicalorversion-history.But for that, it's too late. That'd be a BC break. What we can do is updating the existing documentation.
Comment #4
markdorisonComment #5
markdorisonComment #6
yesct commentedI checked https://www.drupal.org/core/d8-allowed-changes#patch
And I think this can still go in 8.1.x since "API documentation improvements" are allowed.
@markdorison Thanks for looking at this issue.
Adding some more information about what you do when you do a review helps the committers and future reviewers.
(https://www.drupal.org/patch/review)
I looked at the patch, also applied in to look at it in more context and noticed:
these two changes can be more consistent with each other (either use the parenthesis or not).
adding the dot at the end of the sentence is a good standards improvement, cause that line is already being changed by the purpose of this issue.
Maybe this @see was supposed to go on the hunk below for the
\Drupal\rest\LinkManager\RelationLinkManagerInterface::getRelationInternalIds
(or reference getRelationInternalIds, or maybe reference somewhere else)
Cause, where the @see is now, it is referencing itself.
Comment #8
rosinegrean commentedComment #9
rosinegrean commentedUpdate the doc with the mentions in #6
Comment #10
wim leersThis looks great, thanks! :) But let's postpone this on #2758897: Move rest module's "link manager" services to serialization module (which will move it) and #2113345: Define a mechanism for custom link relationships (which may require this to change).
Comment #12
wim leers#2758897: Move rest module's "link manager" services to serialization module and #2113345: Define a mechanism for custom link relationships have landed since I posted #10. Yay! :)
This interface now lives in the
halmodule, so changing the component.Given that this is clearly a trivial/minor docs-only change, demoting to priority.
Comment #15
gábor hojtsyThanks all, committed to both 8.4.x and 8.3.x given that patch releases allow API docs updates as per https://www.drupal.org/core/d8-allowed-changes#patch.