DOC Document upgrading intervention/image #547
Conversation
GuySartorelli
force-pushed
the
pulls/6/intervention-3
branch
3 times, most recently
from
5b33701
to
f0fce23
Compare
GuySartorelli
force-pushed
the
pulls/6/intervention-3
branch
from
f0fce23
to
83b00a7
Compare
GuySartorelli
force-pushed
the
pulls/6/intervention-3
branch
from
83b00a7
to
7423f1c
Compare
|
|
||
| Manipulating animated images takes longer, and results in a larger filesize. | ||
|
|
||
| Because of this, the [`ThumbnailGenerator`](api:SilverStripe\AssetAdmin\Model\ThumbnailGenerator) will provide still images as thumbnails for animated gifs by default. You can change that for a given instance of `ThumbnailGenerator` by passing `true` to the [`setAllowsAnimation()`](api:SilverStripe\AssetAdmin\Model\ThumbnailGenerator::setAllowsAnimation()) method. For example, to allow animated thumbnails for `UploadField`: |
There was a problem hiding this comment.
This also applies to all other PRs to changelogs - we're essentially duplicating docs on doc.silverstripe.org. We should just link to those docs instead of duplicating them in the changelog.
There was a problem hiding this comment.
It's not at all uncommon for us to do this - we often give the most pertinent information directly in the changelog, even if it's also documented elsewhere.
This also applies to all other PRs to changelogs
I assume you want some amount of this removed, can you please indicate how much so I know how much I want to push back on it? :p
There was a problem hiding this comment.
It's not at all uncommon for us to do this - we often give the most pertinent information directly in the changelog, even if it's also documented elsewhere.
Yeah we do ... though just looking at this PR I actually have no idea why we've been doing this. We're quick to call out duplicate documentation in all other scenarios and say "You should just link to the original source" ... I'm not sure why the changelog should be treated any differently.
I assume you want some amount of this removed, can you please indicate how much so I know how much I want to push back on it? :p
Probably everything not in the first section intervention/image has been upgraded from v2 to v3 {#intervention-image} - I'd keep that and say that imagick is now the default if it's available and link to the docs
Most of the rest is duplication of "how to configure it", which is basically straight duplication
I'd also remove the "New API" bit, it just seems unnecessary. Very few people are going to be interested in calling the PHP methods directly, the configuration is much more relevant. If they really are interested in calling PHP methods directly then there's typehinting available anyway.
|
|
||
| Manipulating animated images takes longer, and results in a larger filesize. | ||
|
|
||
| Because of this, the [`ThumbnailGenerator`](api:SilverStripe\AssetAdmin\Model\ThumbnailGenerator) will provide still images as thumbnails for animated gifs by default. You can change that for a given instance of `ThumbnailGenerator` by passing `true` to the [`setAllowsAnimation()`](api:SilverStripe\AssetAdmin\Model\ThumbnailGenerator::setAllowsAnimation()) method. For example, to allow animated thumbnails for `UploadField`: |
There was a problem hiding this comment.
It's not at all uncommon for us to do this - we often give the most pertinent information directly in the changelog, even if it's also documented elsewhere.
Yeah we do ... though just looking at this PR I actually have no idea why we've been doing this. We're quick to call out duplicate documentation in all other scenarios and say "You should just link to the original source" ... I'm not sure why the changelog should be treated any differently.
I assume you want some amount of this removed, can you please indicate how much so I know how much I want to push back on it? :p
Probably everything not in the first section intervention/image has been upgraded from v2 to v3 {#intervention-image} - I'd keep that and say that imagick is now the default if it's available and link to the docs
Most of the rest is duplication of "how to configure it", which is basically straight duplication
I'd also remove the "New API" bit, it just seems unnecessary. Very few people are going to be interested in calling the PHP methods directly, the configuration is much more relevant. If they really are interested in calling PHP methods directly then there's typehinting available anyway.
The changelog is the one thing developers will look at when evaluating what changes they need to make, and it serves double duty as an upgrade guide. Giving all of the most important information in the changelog does two things:
I like the precedent that the important information is included in the changelog even if that same information is provided in other sections of the docs. I see the changelog as "here's all the most important information, including the most relevant info you need for upgrading", whereas the rest of the docs is for future reference or searching for specific information, outside of the context of releases. |
|
I'm not keen to change that precedent now, especially when we don't have a team lead. |
|
We've also got a while 'til this gets released, and if it's anything like CMS 5 we'll spend a fair amount of time going back over the changelog to decide what stays in there, so we can revisit this precedent then, if we haven't already revisited it for a minor release in the meantime (again, when we have a team lead preferably) |
Needs silverstripe/documentation-lint#6 for linting to pass
Issue