docserver: Adds "API scheduled for Chrome version..." text to dev and beta APIs

docserver: Adds "API scheduled for Chrome version..." text to dev and beta APIs

BUG=303975
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267367

[danielj41, 2014-04-23 23:55:56]

This patch adds "API scheduled for Chrome version #" text to dev and beta APIs.
It looks at the dev and beta versions of Chrome and sees if the API is stable at
either of those versions. If so, the API is displayed as "scheduled for" the
earliest version where the API is stable. Is this the correct way to go about
doing this?

[jshumway, 2014-04-25 19:43:10]

https://codereview.chromium.org/255473003/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/api_data_source_test.py (right):

https://codereview.chromium.org/255473003/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source_test.py:146: 'scheduled':
None
Add a test case where 'scheduled' is set

[danielj41, 2014-04-25 21:01:00]

https://codereview.chromium.org/255473003/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/api_data_source_test.py (right):

https://codereview.chromium.org/255473003/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source_test.py:146: 'scheduled':
None
I added a new FakeAvailabilityFinder that returns different data then repeats
the same test. Is it too repetitive?

On 2014/04/25 19:43:10, jshumway wrote:
> Add a test case where 'scheduled' is set

[jshumway, 2014-04-25 21:34:43]

lgtm with style comments

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source_test.py (right):

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source_test.py:187: expected_list
= [
Since only the second item in the list has changed, I would add

```
expected_list[1] = {
  'title': 'Availability',
  'content': [
    {
      'partial': ...,
      'version': 28,
      'scheduled': 28'
    }
  ]
}
```

instead of copying the entire definition.

Maybe add a small comment that you are mutating the expected to output to test
the scheduled feature.

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/availability_finder.py (right):

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:218: return
self._CheckStableAvailability(api_name,
When arguments go past the end of the line but aren't that long by themselves,
use the following style:

```
return really_long_function_name(
    short_arg, short_arg, short_arg, short_arg)
```

so there isn't a big column of code on the right side of the file and it save
vertical space.

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:223:
check_scheduled)
These two arguments can be on the same line under
`file_system_iterator.Descending(`

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:227: return None
The above if else can be

return stable_availability.version if stable_availability else None

[danielj41, 2014-04-25 22:24:48]

This patch adds "API scheduled for Chrome version #" text to dev and beta APIs.
It looks at the dev and beta versions of Chrome and sees if the API is stable at
either of those versions. If so, the API is displayed as "scheduled for" that
version. Is this the correct way to interpret the data?

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source_test.py (right):

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source_test.py:187: expected_list
= [
On 2014/04/25 21:34:43, jshumway wrote:
> Since only the second item in the list has changed, I would add
> 
> ```
> expected_list[1] = {
>   'title': 'Availability',
>   'content': [
>     {
>       'partial': ...,
>       'version': 28,
>       'scheduled': 28'
>     }
>   ]
> }
> ```
> 
> instead of copying the entire definition.
> 
> Maybe add a small comment that you are mutating the expected to output to test
> the scheduled feature.

Done.

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/availability_finder.py (right):

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:218: return
self._CheckStableAvailability(api_name,
On 2014/04/25 21:34:43, jshumway wrote:
> When arguments go past the end of the line but aren't that long by themselves,
> use the following style:
> 
> ```
> return really_long_function_name(
>     short_arg, short_arg, short_arg, short_arg)
> ```
> 
> so there isn't a big column of code on the right side of the file and it save
> vertical space.

Done.

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:223:
check_scheduled)
On 2014/04/25 21:34:43, jshumway wrote:
> These two arguments can be on the same line under
> `file_system_iterator.Descending(`

Done.

https://codereview.chromium.org/255473003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:227: return None
On 2014/04/25 21:34:43, jshumway wrote:
> The above if else can be
> 
> return stable_availability.version if stable_availability else None

Done.

[not at google - send to devlin, 2014-04-25 23:15:58]

nice!

https://codereview.chromium.org/255473003/diff/40001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/branch_utility.py (right):

https://codereview.chromium.org/255473003/diff/40001/chrome/common/extensions...
chrome/common/extensions/docs/server2/branch_utility.py:31: self.scheduled =
scheduled
this isn't really the right place for this. ChannelInfo encapsulates a Chrome
channel, not channel-specific data about an API.

you'll probably need to change GetApiAvailability to return a new
"AvailabilityInfo" type which contains a "channel" and "scheduled" field.

https://codereview.chromium.org/255473003/diff/40001/chrome/common/extensions...
File
chrome/common/extensions/docs/templates/private/intro_tables/dev_message.html
(right):

https://codereview.chromium.org/255473003/diff/40001/chrome/common/extensions...
chrome/common/extensions/docs/templates/private/intro_tables/dev_message.html:4:
{{/content.scheduled}}
the intro table is a better place for this, like

Availability | Dev channel (scheduled for ..)

[danielj41, 2014-04-28 22:54:39]

I created an AvailabilityInfo class that holds a reference to a ChannelInfo
object and a scheduled version number. I moved the "scheduled for..." text out
of the intro table dev/beta partials and into the intro_table.html partial.

[not at google - send to devlin, 2014-04-29 00:19:43]

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source_test.py (right):

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source_test.py:49: return
AvailabilityInfo(ChannelInfo('dev', '1500', 28), 28)
use a different 'version' and 'scheduled' just to be safe (maybe the numbers
have been mixed up or something).

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/availability_finder.py (right):

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:96:
api_info.get('scheduled'))
make explicit args explicit; "scheduled=api_info.get('scheduled')"

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:100:
api_info.get('scheduled'))
ditto

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/private/intro_table.html (right):

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/templates/private/intro_table.html:11:
{{?c.scheduled}}Scheduled for Chrome {{c.scheduled}}.{{/c.scheduled}}
almost. this property is specific to the dev/beta templates so let's put it in
there.

[danielj41, 2014-04-30 20:01:52]

On 2014/04/29 00:19:43, kalman wrote:
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> File chrome/common/extensions/docs/server2/api_data_source_test.py (right):
> 
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> chrome/common/extensions/docs/server2/api_data_source_test.py:49: return
> AvailabilityInfo(ChannelInfo('dev', '1500', 28), 28)
> use a different 'version' and 'scheduled' just to be safe (maybe the numbers
> have been mixed up or something).
> 
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> File chrome/common/extensions/docs/server2/availability_finder.py (right):
> 
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> chrome/common/extensions/docs/server2/availability_finder.py:96:
> api_info.get('scheduled'))
> make explicit args explicit; "scheduled=api_info.get('scheduled')"
> 
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> chrome/common/extensions/docs/server2/availability_finder.py:100:
> api_info.get('scheduled'))
> ditto
> 
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> File chrome/common/extensions/docs/templates/private/intro_table.html (right):
> 
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> chrome/common/extensions/docs/templates/private/intro_table.html:11:
> {{?c.scheduled}}Scheduled for Chrome {{c.scheduled}}.{{/c.scheduled}}
> almost. this property is specific to the dev/beta templates so let's put it in
> there.

Could you please clarify which template file the "scheduled for..." text should
be in? It sounds similar to patch set 3, so I'm not sure what I should do
differently.

[not at google - send to devlin, 2014-04-30 20:30:56]

On 2014/04/30 20:01:52, danielj41 wrote:
> On 2014/04/29 00:19:43, kalman wrote:
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > File chrome/common/extensions/docs/server2/api_data_source_test.py (right):
> > 
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > chrome/common/extensions/docs/server2/api_data_source_test.py:49: return
> > AvailabilityInfo(ChannelInfo('dev', '1500', 28), 28)
> > use a different 'version' and 'scheduled' just to be safe (maybe the numbers
> > have been mixed up or something).
> > 
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > File chrome/common/extensions/docs/server2/availability_finder.py (right):
> > 
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > chrome/common/extensions/docs/server2/availability_finder.py:96:
> > api_info.get('scheduled'))
> > make explicit args explicit; "scheduled=api_info.get('scheduled')"
> > 
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > chrome/common/extensions/docs/server2/availability_finder.py:100:
> > api_info.get('scheduled'))
> > ditto
> > 
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > File chrome/common/extensions/docs/templates/private/intro_table.html
(right):
> > 
> >
>
https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
> > chrome/common/extensions/docs/templates/private/intro_table.html:11:
> > {{?c.scheduled}}Scheduled for Chrome {{c.scheduled}}.{{/c.scheduled}}
> > almost. this property is specific to the dev/beta templates so let's put it
in
> > there.
> 
> Could you please clarify which template file the "scheduled for..." text
should
> be in? It sounds similar to patch set 3, so I'm not sure what I should do
> differently.

Yes you're right, I misread patchset 3 :\ sorry.

[danielj41, 2014-04-30 21:07:10]

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source_test.py (right):

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source_test.py:49: return
AvailabilityInfo(ChannelInfo('dev', '1500', 28), 28)
On 2014/04/29 00:19:43, kalman wrote:
> use a different 'version' and 'scheduled' just to be safe (maybe the numbers
> have been mixed up or something).

Done.

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/availability_finder.py (right):

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:96:
api_info.get('scheduled'))
On 2014/04/29 00:19:43, kalman wrote:
> make explicit args explicit; "scheduled=api_info.get('scheduled')"

Done.

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:100:
api_info.get('scheduled'))
On 2014/04/29 00:19:43, kalman wrote:
> ditto

Done.

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/private/intro_table.html (right):

https://codereview.chromium.org/255473003/diff/80001/chrome/common/extensions...
chrome/common/extensions/docs/templates/private/intro_table.html:11:
{{?c.scheduled}}Scheduled for Chrome {{c.scheduled}}.{{/c.scheduled}}
On 2014/04/29 00:19:43, kalman wrote:
> almost. this property is specific to the dev/beta templates so let's put it in
> there.

Done.

[not at google - send to devlin, 2014-04-30 21:38:22]

lg but before you submit can you rebase and upload that so that I can patch in
the change and have a look?

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
File chrome/common/extensions/docs/server2/availability_finder.py (right):

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
chrome/common/extensions/docs/server2/availability_finder.py:97: else:
cleanup: no else after return

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
chrome/common/extensions/docs/server2/availability_finder.py:100:
scheduled=api_info.get('scheduled'))
though I actually don't think we need to support 'scheduled' from the
api_availabilities.json file, so you don't need to worry about that.

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
File
chrome/common/extensions/docs/templates/private/intro_tables/beta_message.html
(right):

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
chrome/common/extensions/docs/templates/private/intro_tables/beta_message.html:6:
{{/content.scheduled}}
nice. slightly more idiomatic:

{{?content.scheduled}}
(scheduled for Chrome {{content.scheduled}}).
{{:content.scheduled}}
only.
{{/content.scheduled}}

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
File
chrome/common/extensions/docs/templates/private/intro_tables/dev_message.html
(right):

https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
chrome/common/extensions/docs/templates/private/intro_tables/dev_message.html:5:
{{/content.scheduled}}
ditto

[danielj41, 2014-04-30 22:18:53]

On 2014/04/30 21:38:22, kalman wrote:
> lg but before you submit can you rebase and upload that so that I can patch in
> the change and have a look?
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
> File chrome/common/extensions/docs/server2/availability_finder.py (right):
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
> chrome/common/extensions/docs/server2/availability_finder.py:97: else:
> cleanup: no else after return
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
> chrome/common/extensions/docs/server2/availability_finder.py:100:
> scheduled=api_info.get('scheduled'))
> though I actually don't think we need to support 'scheduled' from the
> api_availabilities.json file, so you don't need to worry about that.
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
> File
> chrome/common/extensions/docs/templates/private/intro_tables/beta_message.html
> (right):
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
>
chrome/common/extensions/docs/templates/private/intro_tables/beta_message.html:6:
> {{/content.scheduled}}
> nice. slightly more idiomatic:
> 
> {{?content.scheduled}}
> (scheduled for Chrome {{content.scheduled}}).
> {{:content.scheduled}}
> only.
> {{/content.scheduled}}
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
> File
> chrome/common/extensions/docs/templates/private/intro_tables/dev_message.html
> (right):
> 
>
https://codereview.chromium.org/255473003/diff/100001/chrome/common/extension...
>
chrome/common/extensions/docs/templates/private/intro_tables/dev_message.html:5:
> {{/content.scheduled}}
> ditto

I merged from master and made the style changes. I've had to test it using
start_dev_server with local template files, because preview.py seemed to be
using different availability data.

[not at google - send to devlin, 2014-04-30 22:19:43]

Yes the preview server only serves local data.

[not at google - send to devlin, 2014-04-30 22:26:14]

lgtm

[danielj41, 2014-04-30 22:27:47]

The CQ bit was checked by danielj41@gmail.com

[commit-bot: I haz the power, 2014-04-30 22:28:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danielj41@gmail.com/255473003/120001

[commit-bot: I haz the power, 2014-04-30 22:47:33]

Change committed as 267367