[WIP] Documenting the SeoBundle #442
Conversation
| ), | ||
| ); | ||
|
|
||
| .. todo |
There was a problem hiding this comment.
I don't really get the concepts of this whole thing. Why can't we combine the default and the document values in the getSeo*() methods? /cc @dbu @ElectricMaxxx
There was a problem hiding this comment.
You mean injecting the more than the current metadata into the getSeo*() extraction methods to let the implementation decide how to concatenate everything together?
This would mean to give more work (for the devolper) into the extraction methods, would that be ok?
But why are commenting that line?
There was a problem hiding this comment.
But i think you are right, we should give the developer that whants to handle the default values the chance to do it, but others should get a simple return $this->getMyTitle() solution.
Btw. i would like to have a list of possible getters for the SeoTitleReadExtractor (that one that guesses title getter) in the config.
|
@ElectricMaxxx do you have time to wrap this up, now that the bundle stabilized? |
|
@dbu Yes i will do. But can be tomorrow evening. |
|
I'll do this tomorrow :) (since it's my PR) |
|
👍 then it won't that hard for you to review. |
|
Ok, I've finished rewriting this documentation. Now, all that's left is documenting the last 2 merged PRs. |
| +-----------------------------------+---------------------------+----------------------------------------------+ | ||
| | ``SeoTitleReadInterface`` | ``getSeoTitle()`` | Returns the page title | | ||
| +-----------------------------------+---------------------------+----------------------------------------------+ | ||
| | - | ``getTitle()`` | If the document has a ``getTitle()`` method, | |
There was a problem hiding this comment.
this extractor is not active by default, right? can we enable / disable each extractor individually?
There was a problem hiding this comment.
I thought it was active, we have now way of activating/deactivating extractors. That's why I introduced mappings instead extractor interfaces: symfony-cmf/seo-bundle#133
There was a problem hiding this comment.
okay, just checked and you are right. good enough for 1.0 ;-)
|
extractors.rst is not in a toc tree, breaking the build. |
| ), | ||
| ); | ||
|
|
||
| Defining a Default |
There was a problem hiding this comment.
Let's change it to "Defining Default SEO Data"
|
@dbu I'm going to wrap this up this evening. |
|
Finished :) Let's wait what Travis thinks about it and I would really love if someone could review the seo_aware documentation, since I don't have a very good Doctrine knowledge... |
|
I hope i will have a look this night. On 05/13/2014 08:48 PM, Wouter J wrote:
|
| can do the work for you. Extractors are executed when an object implements a | ||
| specific interface. The method required by that interface will return the | ||
| value for the specific SEO data. The extractor will then update the | ||
| ``SeoMetadata`` object for the current object with the returned value. |
There was a problem hiding this comment.
this relies on the RoutingBundle enhancers putting the main content into the CONTENT_KEY field in parameters. i think we should somehow mention that here.
There was a problem hiding this comment.
indeed, added a paragraph about it
|
okay, i went through the doc now. coming good, but a few things to clean up. |
|
Ok, that's the final push to this PR. Thanks @dbu and @ElectricMaxxx for your great review job! If you have anything more to comment, please open a PR and fix it directly. I'm now going to merge it so we can announce 1.1 tonight |
…umentation [WIP] Documenting the SeoBundle
|
awesome, thanks a lot! |
Replaces #438
Todo