Added JsonLd CreativeWork #1735
Conversation
|
Looks good to me! Could you please add a change log entry (Add new section And also think there should be a better description of CreativeWork. We should people tell what they should cover with this CreativeWork like Its for
In the description of the class. This would people help. I think we should maybe also extend the existing jsonld classes with such descriptions as well. |
|
|
||
| /** | ||
| * JsonLd - Creative Work | ||
| * |
There was a problem hiding this comment.
Better description for people, what this can cover. like blog .. .etc.
core/web/jsonld/ArticleTrait.php
Outdated
| } | ||
|
|
||
| /** | ||
| * Articles may belong to one or more 'sections' in a magazine or newspaper, such as Sports, Lifestyle, etc. |
There was a problem hiding this comment.
@JohnnyMcWeed The private will not be shown anywhere (not in IDE nor on luya.io/api) so the information should be stored on the getter method. You can even fully remove any phpdoc for the private var. (for example: https://github.com/luyadev/luya/blob/master/core/tag/BaseTag.php)
There was a problem hiding this comment.
You're right. I'll change that for all classes in the next few days.
core/web/JsonLd.php
Outdated
| * | ||
| * @return \luya\web\jsonld\Article | ||
| */ | ||
| public static function article(array $config = []) |
core/web/JsonLd.php
Outdated
| * @param array $config Optional config array to provided blog posting data via setter methods. | ||
| * | ||
| * @return \luya\web\jsonld\BlogPosting | ||
| */ |
There was a problem hiding this comment.
(github just marked the user @ since :-) but i think you know what i mean)
| * | ||
| * @param array $config Optional config array to provided person data via setter methods. | ||
| * @param array $config Optional config array to provided creative work data via setter methods. |
core/web/JsonLd.php
Outdated
| * Register new Live Blog Posting. | ||
| * | ||
| * @param array $config Optional config array to provided live blog posting data via setter methods. | ||
| * |
| /** | ||
| * Register new Social Media Posting. | ||
| * | ||
| * @param array $config Optional config array to provided social media posting data via setter methods. |
There was a problem hiding this comment.
I'll walk through and add it in the next few days
core/web/jsonld/ArticleTrait.php
Outdated
|
|
||
| /** | ||
| * @param string $articleSection | ||
| * @return Article|ArticleTrait |
There was a problem hiding this comment.
The phpdoc return except an object of type. So we could typehint this for $articleSection like Article $articleSection - what do you think? or are there any other optiosn available?
There was a problem hiding this comment.
I think the hinting right now makes more sense, as return "Article $articleSection" could confuse in case of what is returned. The return is the article (or any object when the trait is used). Or what do you think?
|
@JohnnyMcWeed i almost forgot something very important. we would need to have a very small unit test for the new classes. sorry |
|
I already did some tests but not yet pushed. Could you make a "complete" test for Thing? Like that I have a class I can check what it needs to test all methods, as I can't see the result. |
|
@JohnnyMcWeed sure i can provide such an option. But to keep it simple just add something like this: https://github.com/luyadev/luya/blob/master/tests/core/web/JsonLdTest.php#L42-L52 |
|
Already did these but not yet pushed ;) |
What are you changing/introducing
Added JsonLd CreativeWork (Interface, Trait and Class)