Issue 227073006: Added a policy writer for iOS Plists.

Issue 227073006: Added a policy writer for iOS Plists. (Closed)

Created:
6 years, 8 months ago by Joao da Silva

Modified:
6 years, 8 months ago

Reviewers:
pastarmovj

CC:
grit-developer_googlegroups.com

Base URL:
https://chromium.googlesource.com/external/grit-i18n.git@master

Visibility:
Public.

More Reviews

Description

Added a policy writer for iOS Plists. BUG=360508 R=pastarmovj@chromium.org Committed: https://code.google.com/p/grit-i18n/source/detail?r=159

Patch Set 2 : Generate correct examples for Mac on the doc_writer #

Patch Set 4 : rebase #

Created: 6 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+74 lines, -97 lines)			Patch
M	grit/format/policy_templates/policy_template_generator.py	View	1 2 3	3 chunks	+0 lines, -17 lines	0 comments	Download
M	grit/format/policy_templates/writers/adml_writer_unittest.py	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	grit/format/policy_templates/writers/admx_writer_unittest.py	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	grit/format/policy_templates/writers/doc_writer.py	View	1 2 3	2 chunks	+26 lines, -35 lines	0 comments	Download
M	grit/format/policy_templates/writers/json_writer.py	View	1 2 3	2 chunks	+4 lines, -20 lines	0 comments	Download
M	grit/format/policy_templates/writers/json_writer_unittest.py	View	1 2 3	2 chunks	+6 lines, -6 lines	0 comments	Download
M	grit/format/policy_templates/writers/plist_writer.py	View	1 2 3	1 chunk	+5 lines, -2 lines	0 comments	Download
M	grit/format/policy_templates/writers/reg_writer.py	View	1 2 3	3 chunks	+6 lines, -5 lines	0 comments	Download
M	grit/format/policy_templates/writers/reg_writer_unittest.py	View	1 2 3	2 chunks	+6 lines, -6 lines	0 comments	Download
M	grit/format/policy_templates/writers/xml_formatted_writer.py	View	1 2 3	2 chunks	+6 lines, -2 lines	0 comments	Download
M	grit/test_suite_all.py	View	1 2 3	2 chunks	+9 lines, -0 lines	0 comments	Download
M	grit/tool/build.py	View	1 2 3	1 chunk	+4 lines, -2 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

pastarmovj

I would stamp this but please consider my comment on the json import first. https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates/writers/ios_plist_writer.py ...

6 years, 8 months ago (2014-04-07 09:49:44 UTC) #2

Joao da Silva

Thanks for the comments, PTAL. https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates/writers/ios_plist_writer.py File grit/format/policy_templates/writers/ios_plist_writer.py (right): https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates/writers/ios_plist_writer.py#newcode64 grit/format/policy_templates/writers/ios_plist_writer.py:64: support_on['until_version'] == '' and ...

6 years, 8 months ago (2014-04-07 10:10:48 UTC) #3

Thanks for the comments, PTAL.

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
File grit/format/policy_templates/writers/ios_plist_writer.py (right):

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
grit/format/policy_templates/writers/ios_plist_writer.py:64:
support_on['until_version'] == '' and
On 2014/04/07 09:49:44, pastarmovj wrote:
> I guess this assumes that we are never going to put the current version as
> until_version and only put this flag on already deprecated ones? 

Yes, usually when we remove support we also set the 'until_version' to the
previous milestone.

Note that this isn't a documentation writer (which could include deprecated and
removed policies), and should only generate examples that are valid for the
current milestone.

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
File grit/format/policy_templates/writers/ios_plist_writer_unittest.py (right):

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
grit/format/policy_templates/writers/ios_plist_writer_unittest.py:183: def
testManagedBookmarks(self):
On 2014/04/07 09:49:44, pastarmovj wrote:
> Maybe just clal it testDict? From the perspective of GRIT bookmarks is a
little
> to high-level concept.

Done.

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
grit/format/policy_templates/writers/ios_plist_writer_unittest.py:186: '''[
On 2014/04/07 09:49:44, pastarmovj wrote:
> nit:Can you put the open bracket on a new line otherwise the alignment seems
> wonky?

Done.

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
File grit/format/policy_templates/writers/json_writer.py (right):

https://codereview.chromium.org/227073006/diff/1/grit/format/policy_templates...
grit/format/policy_templates/writers/json_writer.py:6: import json
On 2014/04/07 09:49:44, pastarmovj wrote:
> Not 100% sure but wasn't there some issue or incompatibility between the json
> impl on different python versions which caused some trouble in the past - like
> unstable ordering or some such? I think this is the reason to write the files
> manually in this way.

Yes, that was fixed by https://codereview.chromium.org/14940026.

I've now reverted most of that and instead made the generators write json with
sorted keys (look for the sort_keys=True parameter to json.dumps). The unittest
have been updated to check the text output with sorted keys too.

Joao da Silva

6 years, 8 months ago (2014-04-07 13:38:38 UTC) #5

Message was sent while issue was closed.

Committed patchset #4 manually as r159 (presubmit successful).

Expand Messages | Collapse Messages