Issue 29628724: Issue 5521 - Restructure CMS documentation to make it easier to use and extend

Issue 29628724: Issue 5521 - Restructure CMS documentation to make it easier to use and extend (Closed)

Created:
Dec. 4, 2017, 9:59 a.m. by ire

Modified:
Dec. 16, 2017, 9:43 a.m.

Reviewers:
Vasily Kuznetsov

CC:
Jon Sonesen, juliandoucette

Base URL:
https://hg.adblockplus.org/cms

Visibility:
Public.

Description

Issue 5521 - Restructure CMS documentation to make it easier to use and extend

Patch Set 1 #

Total comments: 12

Patch Set 2 : Add initial API docs #

Total comments: 1

Created: Dec. 13, 2017, 11:08 a.m.

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+333 lines, -297 lines)			Patch
M	README.md	View	1	2 chunks	+23 lines, -297 lines	0 comments	Download
A	docs/api/filters.md	View	1	1 chunk	+16 lines, -0 lines	0 comments	Download
A	docs/api/functions.md	View	1	1 chunk	+11 lines, -0 lines	0 comments	Download
A	docs/api/variables.md	View	1	1 chunk	+9 lines, -0 lines	0 comments	Download
A	docs/content/filters.md	View	1	1 chunk	+8 lines, -0 lines	0 comments	Download
A	docs/content/globals.md	View	1	1 chunk	+7 lines, -0 lines	0 comments	Download
A	docs/content/includes.md	View	1	1 chunk	+10 lines, -0 lines	1 comment	Download
A	docs/content/locales.md	View	1	1 chunk	+19 lines, -0 lines	0 comments	Download
A	docs/content/pages.md	View	1	1 chunk	+83 lines, -0 lines	0 comments	Download
A	docs/content/settings.md	View	1	1 chunk	+21 lines, -0 lines	0 comments	Download
A	docs/content/static.md	View	1	1 chunk	+8 lines, -0 lines	0 comments	Download
A	docs/content/templates.md	View	1	1 chunk	+13 lines, -0 lines	0 comments	Download
A	docs/usage/generate-static-files.md	View		1 chunk	+26 lines, -0 lines	0 comments	Download
A	docs/usage/standalone-test-server.md	View		1 chunk	+15 lines, -0 lines	0 comments	Download
A	docs/usage/syncing-translations.md	View		1 chunk	+35 lines, -0 lines	0 comments	Download
A	docs/usage/test-server.md	View	1	1 chunk	+29 lines, -0 lines	0 comments	Download

Messages

Total messages: 9

Expand All Messages | Collapse All Messages

ire

@Vasily Not sure if this is what you had in mind, but here's my shot ...

Dec. 4, 2017, 10:17 a.m. (2017-12-04 10:17:15 UTC) #2

juliandoucette

I'm going to sit this one out - unless you ask me something specifically

Dec. 4, 2017, 1:02 p.m. (2017-12-04 13:02:54 UTC) #3

Vasily Kuznetsov

Hi Ire, This looks pretty good. I quite like the way you reorganized it. I ...

Dec. 12, 2017, 3:35 p.m. (2017-12-12 15:35:25 UTC) #4

ire

On 2017/12/12 15:35:25, Vasily Kuznetsov wrote: > Hi Ire, > > This looks pretty good. ...

Dec. 13, 2017, 11:10 a.m. (2017-12-13 11:10:25 UTC) #5

On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> Hi Ire,
> 
> This looks pretty good. I quite like the way you reorganized it.
> 
> I have a few proposals for minor changes (see below) -- let me know what you
> think about them.
> 
> Cheers,
> Vasily

Thanks Vasily! I've uploaded a new patch and responded to your comments.

https://codereview.adblockplus.org/29628724/diff/29628725/README.md
File README.md (right):

https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode46
README.md:46: - [Configuration (`settings.ini`)](docs/structure/settings.md)
On 2017/12/12 15:35:24, Vasily Kuznetsov wrote:
> I think I prefer to call the folder `docs/content` instead of `docs/structure`
> -- seems more intuitively clear what's in it.

I agree. Done.

https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode48
README.md:48: - [Custom Jinja2 filters (`filters`)](docs/structure/filters.md)
On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> This link is broken, the file is called `filter.md`.

Done.

https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode54
README.md:54: - API
On 2017/12/12 15:35:24, Vasily Kuznetsov wrote:
> On 2017/12/04 10:17:13, ire wrote:
> > In the future I hope for the API documentation to be here (which will list
all
> > the CMS methods we can use). This bullet point can be removed until that
> > documentation is added.
> 
> Yeah, we need API documentation. Perhaps the part of `pages.md` that talks
about
> variables, functions and filters available to templates could for the initial
> version of this section. And then it could be linked from `pages.md`.

I like this idea. 

I arranged this into 3 files - variables, filters, and functions. I don't know
if this is the ideal way to categorise them, I think organising them by topic
may be better, e.g. "translation", etc. But until we have a fuller
documentation, I think this could work. Let me know what you think.

https://codereview.adblockplus.org/29628724/diff/29628725/docs/structure/stat...
File docs/structure/static.md (right):

https://codereview.adblockplus.org/29628724/diff/29628725/docs/structure/stat...
docs/structure/static.md:5: `/css/foo.css`.
On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> Maybe we can add a link to the end of `locales.md` where we talk about
> localizable static files?

Good idea. Done.

https://codereview.adblockplus.org/29628724/diff/29628725/docs/usage/test-ser...
File docs/usage/test-server.md (right):

https://codereview.adblockplus.org/29628724/diff/29628725/docs/usage/test-ser...
docs/usage/test-server.md:28: There you should generate static files as
explained below.
On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> I think it would be better to add a link to `generate-static-files.md` here
now.
> What do you think?

Good idea. Done.

https://codereview.adblockplus.org/29628724/diff/29637558/docs/content/includ...
File docs/content/includes.md (right):

https://codereview.adblockplus.org/29628724/diff/29637558/docs/content/includ...
docs/content/includes.md:1: # Include files #
I also separated the part in pages.md that talked about include files and moved
it here

Vasily Kuznetsov

On 2017/12/13 11:10:25, ire wrote: > On 2017/12/12 15:35:25, Vasily Kuznetsov wrote: > > Hi ...

Dec. 13, 2017, 5:38 p.m. (2017-12-13 17:38:21 UTC) #6

On 2017/12/13 11:10:25, ire wrote:
> On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > Hi Ire,
> > 
> > This looks pretty good. I quite like the way you reorganized it.
> > 
> > I have a few proposals for minor changes (see below) -- let me know what you
> > think about them.
> > 
> > Cheers,
> > Vasily
> 
> Thanks Vasily! I've uploaded a new patch and responded to your comments.
> 
> https://codereview.adblockplus.org/29628724/diff/29628725/README.md
> File README.md (right):
> 
> https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode46
> README.md:46: - [Configuration (`settings.ini`)](docs/structure/settings.md)
> On 2017/12/12 15:35:24, Vasily Kuznetsov wrote:
> > I think I prefer to call the folder `docs/content` instead of
`docs/structure`
> > -- seems more intuitively clear what's in it.
> 
> I agree. Done.
> 
> https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode48
> README.md:48: - [Custom Jinja2 filters (`filters`)](docs/structure/filters.md)
> On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > This link is broken, the file is called `filter.md`.
> 
> Done.
> 
> https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode54
> README.md:54: - API
> On 2017/12/12 15:35:24, Vasily Kuznetsov wrote:
> > On 2017/12/04 10:17:13, ire wrote:
> > > In the future I hope for the API documentation to be here (which will list
> all
> > > the CMS methods we can use). This bullet point can be removed until that
> > > documentation is added.
> > 
> > Yeah, we need API documentation. Perhaps the part of `pages.md` that talks
> about
> > variables, functions and filters available to templates could for the
initial
> > version of this section. And then it could be linked from `pages.md`.
> 
> I like this idea. 
> 
> I arranged this into 3 files - variables, filters, and functions. I don't know
> if this is the ideal way to categorise them, I think organising them by topic
> may be better, e.g. "translation", etc. But until we have a fuller
> documentation, I think this could work. Let me know what you think.
> 
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/structure/stat...
> File docs/structure/static.md (right):
> 
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/structure/stat...
> docs/structure/static.md:5: `/css/foo.css`.
> On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > Maybe we can add a link to the end of `locales.md` where we talk about
> > localizable static files?
> 
> Good idea. Done.
> 
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/usage/test-ser...
> File docs/usage/test-server.md (right):
> 
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/usage/test-ser...
> docs/usage/test-server.md:28: There you should generate static files as
> explained below.
> On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > I think it would be better to add a link to `generate-static-files.md` here
> now.
> > What do you think?
> 
> Good idea. Done.
> 
>
https://codereview.adblockplus.org/29628724/diff/29637558/docs/content/includ...
> File docs/content/includes.md (right):
> 
>
https://codereview.adblockplus.org/29628724/diff/29637558/docs/content/includ...
> docs/content/includes.md:1: # Include files #
> I also separated the part in pages.md that talked about include files and
moved
> it here

Thanks Ire. I still have some minor changes and I think linking could be
improved, but I think it would be more efficient to do it by me changing the
sources rather than in a context of a review. I would suggest to land this
review and close the ticket and then I can add my changes in another ticket or
in a No-issue change. LGTM.

ire

On 2017/12/13 17:38:21, Vasily Kuznetsov wrote: > On 2017/12/13 11:10:25, ire wrote: > > On ...

Dec. 13, 2017, 7:32 p.m. (2017-12-13 19:32:47 UTC) #7

On 2017/12/13 17:38:21, Vasily Kuznetsov wrote:
> On 2017/12/13 11:10:25, ire wrote:
> > On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > > Hi Ire,
> > > 
> > > This looks pretty good. I quite like the way you reorganized it.
> > > 
> > > I have a few proposals for minor changes (see below) -- let me know what
you
> > > think about them.
> > > 
> > > Cheers,
> > > Vasily
> > 
> > Thanks Vasily! I've uploaded a new patch and responded to your comments.
> > 
> > https://codereview.adblockplus.org/29628724/diff/29628725/README.md
> > File README.md (right):
> > 
> >
https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode46
> > README.md:46: - [Configuration (`settings.ini`)](docs/structure/settings.md)
> > On 2017/12/12 15:35:24, Vasily Kuznetsov wrote:
> > > I think I prefer to call the folder `docs/content` instead of
> `docs/structure`
> > > -- seems more intuitively clear what's in it.
> > 
> > I agree. Done.
> > 
> >
https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode48
> > README.md:48: - [Custom Jinja2 filters
(`filters`)](docs/structure/filters.md)
> > On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > > This link is broken, the file is called `filter.md`.
> > 
> > Done.
> > 
> >
https://codereview.adblockplus.org/29628724/diff/29628725/README.md#newcode54
> > README.md:54: - API
> > On 2017/12/12 15:35:24, Vasily Kuznetsov wrote:
> > > On 2017/12/04 10:17:13, ire wrote:
> > > > In the future I hope for the API documentation to be here (which will
list
> > all
> > > > the CMS methods we can use). This bullet point can be removed until that
> > > > documentation is added.
> > > 
> > > Yeah, we need API documentation. Perhaps the part of `pages.md` that talks
> > about
> > > variables, functions and filters available to templates could for the
> initial
> > > version of this section. And then it could be linked from `pages.md`.
> > 
> > I like this idea. 
> > 
> > I arranged this into 3 files - variables, filters, and functions. I don't
know
> > if this is the ideal way to categorise them, I think organising them by
topic
> > may be better, e.g. "translation", etc. But until we have a fuller
> > documentation, I think this could work. Let me know what you think.
> > 
> >
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/structure/stat...
> > File docs/structure/static.md (right):
> > 
> >
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/structure/stat...
> > docs/structure/static.md:5: `/css/foo.css`.
> > On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > > Maybe we can add a link to the end of `locales.md` where we talk about
> > > localizable static files?
> > 
> > Good idea. Done.
> > 
> >
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/usage/test-ser...
> > File docs/usage/test-server.md (right):
> > 
> >
>
https://codereview.adblockplus.org/29628724/diff/29628725/docs/usage/test-ser...
> > docs/usage/test-server.md:28: There you should generate static files as
> > explained below.
> > On 2017/12/12 15:35:25, Vasily Kuznetsov wrote:
> > > I think it would be better to add a link to `generate-static-files.md`
here
> > now.
> > > What do you think?
> > 
> > Good idea. Done.
> > 
> >
>
https://codereview.adblockplus.org/29628724/diff/29637558/docs/content/includ...
> > File docs/content/includes.md (right):
> > 
> >
>
https://codereview.adblockplus.org/29628724/diff/29637558/docs/content/includ...
> > docs/content/includes.md:1: # Include files #
> > I also separated the part in pages.md that talked about include files and
> moved
> > it here
> 
> Thanks Ire. I still have some minor changes and I think linking could be
> improved, but I think it would be more efficient to do it by me changing the
> sources rather than in a context of a review. I would suggest to land this
> review and close the ticket and then I can add my changes in another ticket or
> in a No-issue change. LGTM.

Sounds good to me :)

I don't have push access to the cms repository, so would you be able to commit
and push this for me?

Vasily Kuznetsov

> I don't have push access to the cms repository, so would you be able ...

Dec. 15, 2017, 4:52 p.m. (2017-12-15 16:52:30 UTC) #8

ire

Dec. 16, 2017, 9:43 a.m. (2017-12-16 09:43:25 UTC) #9

Message was sent while issue was closed.

On 2017/12/15 16:52:30, Vasily Kuznetsov wrote:
> > I don't have push access to the cms repository, so would you be able to
commit
> > and push this for me?
> 
> Done!
> Thanks a lot for you time working on this. 
> Feel free to close the review and the ticket.
> 
> Cheers,
> Vasily

No problem at all :)

Expand All Messages | Collapse All Messages