Closed
Bug 1467183
Opened 6 years ago
Closed 6 years ago
Write l10n migration documentation
Categories
(Core :: Internationalization, enhancement, P3)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: flod)
Details
Attachments
(1 file)
One of the lessons learned during the Preferences post-mortem was that we should do better job at explaining ourselves around migrations. Trying to capture devs frame of mind, the migration docs should explain: - what is the problem at hand (migrating to Fluent leaves behind work of 100+ l10n communities) - how migrations solve this problem - how migrations fit into a migration cycle (write a patch, attach migration, land together, we'll do migrations, when the migration script can be removed) - how to write a migration - easy case - common outliers: concatenation, merging two strings into compound message, introducing plurals, introducing -brand-name reference etc. - how test migration - who to ask for help
Reporter | ||
Updated•6 years ago
|
Assignee: nobody → francesco.lodolo
Priority: -- → P3
Assignee | ||
Comment 1•6 years ago
|
||
@zibi Where do you think this doc should live? I think it would be long enough to need its own separate file in https://firefox-source-docs.mozilla.org/intl/l10n/docs/l10n/index.html
Reporter | ||
Comment 2•6 years ago
|
||
Yup, new file in intl/l10n/docs :)
Assignee | ||
Comment 3•6 years ago
|
||
I feel like I've made good progress on the doc, and it's in good shape for review. There's one point that I'm missing, and that's at least a link to a document explaining the AST at high level, but it doesn't look like such a document exists? I thought of linking to the PlayGround, and look at the AST there, but the amount of information is scary compared to what a developer needs to write migrations. I might be also misusing some terminology, e.g. Transforms. I've looked at the comments in some fluent-migration Python files (transforms.py in particular), and while I can write migration easily for most cases, I can't understand much of those comments (Patterns, Placeable, etc.) :)
Comment hidden (mozreview-request) |
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review256312 This is a pretty good start, I found a couple of bugs, though. The details of how migrations are applied might come in handy after the testing part, to give developers an idea that they're actually doing the real deal. ::: intl/l10n/docs/fluent_migrations.rst:22 (Diff revision 1) > +to localize hundreds of strings from scratch. > + > +`Fluent Migration`_ is a Python library designed to solve this specific problem: > +it allows to migrate legacy translations from `.dtd` and `.properties` files, > +not only moving strings and transforming them as needed to adapt to the `FTL` > +syntax, but also replicating in VCS the history of each string. replicating "blame" for each string in VCS ::: intl/l10n/docs/fluent_migrations.rst:38 (Diff revision 1) > +__ https://hg.mozilla.org/mozilla-central/file/default/python/l10n/fluent_migrations > + > +When a feature is migrated to Fluent, a migration recipe should be attached to > +the same patch that adds new strings to `.ftl` files. > + > +Once the patch lands in mozilla-central, l10n-drivers will perform a series of I'd move 38-49 to the very bottom, just for the curious. I think this is too much detail upfront. ::: intl/l10n/docs/fluent_migrations.rst:120 (Diff revision 1) > + def migrate(ctx): > + """Bug 1411707 - Migrate the findbar XBL binding to a Custom Element, part {index}.""" > + > + ctx.add_transforms( > + "toolkit/toolkit/main-window/findbar.ftl", > + "toolkit/toolkit/main-window/findbar.ftl", I wouldn't mind if this was just `toolkit/main-window/findbar.ftl`. Documenting `bla/bla` seems to set a bad incentive. ::: intl/l10n/docs/fluent_migrations.rst:173 (Diff revision 1) > + > + > +This method of writing migration recipes allows to take the original FTL > +strings, and simply replace the value of each message with a :python:`COPY` > +Transform. :python:`transforms_from` takes care of converting the FTL-like > +syntax into an array of FTL messages, unfortunately it’s suitable only for s/unfortunately/something else/ ... FTL messages. This works well for simple copies. When the fluent strings are a ...., the full expressiveness of Fluent migrations comes to help. Or so. ::: intl/l10n/docs/fluent_migrations.rst:178 (Diff revision 1) > +syntax into an array of FTL messages, unfortunately it’s suitable only for > +cases where legacy strings can be copied without any changes. > + > +The example above is equivalent to the following syntax, which requires a deeper > +understanding of the underlying AST structure, and it’s also the only syntax > +available for more complex recipes: I'd remove the pessimistic latter half of that sentence here. ::: intl/l10n/docs/fluent_migrations.rst:208 (Diff revision 1) > +:js:`findbar-next`. A message can have an array of attributes, each with an ID > +and a value: in this case there is only one attribute, with ID :js:`tooltiptext` > +and :js:`value` copied from the legacy string. > + > +Notice how both the ID of the message and the ID of the attribute are > +defined as an :python:`FTL.Identifier`, not simply as a string. Watch out for https://github.com/projectfluent/fluent/issues/144. ::: intl/l10n/docs/fluent_migrations.rst:215 (Diff revision 1) > + > +.. tip:: > + > + It’s possible to concatenate arrays of Transforms manually defined, like in > + the last example, with those coming from :python:`transforms_from`, by using > + the :python:`+` operator. Should we recommend this? I wonder if just doing multiple ctx.add_transforms is easier to digest? ::: intl/l10n/docs/fluent_migrations.rst:387 (Diff revision 1) > +.. code-block:: properties > + > + # LOCALIZATION NOTE (disableContainersOkButton): Semi-colon list of plural forms. > + # See: http://developer.mozilla.org/en/docs/Localization_and_Plurals > + # #S is the number of container tabs > + disableContainersOkButton = Close #S Container Tab;Close #S Container Tabs #1 ::: intl/l10n/docs/fluent_migrations.rst:416 (Diff revision 1) > + "disableContainersOkButton", > + EXTERNAL_ARGUMENT("tabCount"), > + lambda text: REPLACE_IN_TEXT( > + text, > + { > + "#S": EXTERNAL_ARGUMENT("tabCount") #1 ::: intl/l10n/docs/fluent_migrations.rst:424 (Diff revision 1) > + ) > + ) > + > + > +The `PLURALS` Transform will take care of creating the correct number of plural > +categories for each language. Notice how `#S` is replaced for each of these #1 ::: intl/l10n/docs/fluent_migrations.rst:514 (Diff revision 1) > + > + > +How to Test Migration Recipes > +============================= > + > +Unfortunately, testing migration recipes requires several manual steps. We plan Link to bug 1353680 here? ::: intl/l10n/docs/fluent_migrations.rst:524 (Diff revision 1) > +1. Install Fluent Migration > +--------------------------- > + > +The first step is to install the `Fluent Migration`_ Python library. It’s > +currently not available as a package, so the repository must be cloned locally > +and installed manually, e.g. with :bash:`pip install -e .`. Maybe we should just fix that ;-) ::: intl/l10n/docs/fluent_migrations.rst:548 (Diff revision 1) > + > + hg clone https://hg.mozilla.org/l10n/gecko-strings en-US > + cp -r en-US test > + > + > +3. Add New FTL Strings to The Local en-US Repository I'd not try to do title case here. I think that New, The, Local might not want that.
Attachment #8984195 -
Flags: review?(l10n) → review-
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review256306 I didn't have time to finish reviwing all of it, but it looks amazing and I'm sure it will be very useful going forward. Great work, flod! I finished reviwng around line 200, and I need to go now, so I'm going to submit this unfinished review and come back again tomorrow. Please treat my comments as suggestions and feel free to resolve them if you disagree. ::: intl/l10n/docs/fluent_migrations.rst:15 (Diff revision 1) > +================================== > +Migrating Legacy Strings to Fluent > +================================== > + > +Firefox is a project localized in over 100 languages. As the code for existing > +features moves away from the old localization systems and start using `Fluent`_, Grammar: starts ::: intl/l10n/docs/fluent_migrations.rst:16 (Diff revision 1) > +Migrating Legacy Strings to Fluent > +================================== > + > +Firefox is a project localized in over 100 languages. As the code for existing > +features moves away from the old localization systems and start using `Fluent`_, > +we need to ensure that we don’t lose existing translations, forcing contributors ..that we don't lose existing translations which would have the adverse effect of... Otherwise "forcing" might be understood as a qualifier to "ensure". ::: intl/l10n/docs/fluent_migrations.rst:22 (Diff revision 1) > +to localize hundreds of strings from scratch. > + > +`Fluent Migration`_ is a Python library designed to solve this specific problem: > +it allows to migrate legacy translations from `.dtd` and `.properties` files, > +not only moving strings and transforming them as needed to adapt to the `FTL` > +syntax, but also replicating in VCS the history of each string. ...syntax. It also preserves some attribution history for each string in the VCS. ::: intl/l10n/docs/fluent_migrations.rst:35 (Diff revision 1) > +involved, transformations to apply, etc. These recipes are stored in > +`mozilla-central`__. > + > +__ https://hg.mozilla.org/mozilla-central/file/default/python/l10n/fluent_migrations > + > +When a feature is migrated to Fluent, a migration recipe should be attached to When part of Firefox's UI is migrated... ::: intl/l10n/docs/fluent_migrations.rst:45 (Diff revision 1) > + > + - New Fluent strings land in `mozilla-central`, together with a migration > + recipe. > + - `gecko-strings-quarantine`_ – a unified repository including strings > + for all shipping versions of Firefox, used as a buffer before exposing > + strings to localizers – is updated to include the new strings. I'd try to rephrase this sentence in active voice: The new strings lands in gsq, a unified... ::: intl/l10n/docs/fluent_migrations.rst:49 (Diff revision 1) > + for all shipping versions of Firefox, used as a buffer before exposing > + strings to localizers – is updated to include the new strings. > + - Migration recipes are run against all l10n repositories, migrating strings > + from old to new files, and storing them in VCS. > + - New en-US strings are pushed to the official `gecko-strings`_ repository > + used by localization tools, and exposed to all localizers. Perhaps add an explicit step here which says that after this step, the migrations end their life cycle? I know this is explained in the paragraph below, but I think it would make sense to make it in to the last bullet point on this list. ::: intl/l10n/docs/fluent_migrations.rst:154 (Diff revision 1) > + > +In this case there is only one Transform that migrates the string with ID > +:js:`next.tooltip` from :bash:`toolkit/chrome/global/findbar.dtd`, and > +inject it in the FTL fragment. The :python:`COPY` Transform allows to copy the > +string from an existing file as is, while :python:`from_path` is used to avoid > +repeating the same path multiple times, making the recipe less readable. Without Here again, I'd rather say "making the recipe more readable" because I associate this phrase with "is used to avoid" :) Am I reading this wrong? ::: intl/l10n/docs/fluent_migrations.rst:173 (Diff revision 1) > + > + > +This method of writing migration recipes allows to take the original FTL > +strings, and simply replace the value of each message with a :python:`COPY` > +Transform. :python:`transforms_from` takes care of converting the FTL-like > +syntax into an array of FTL messages, unfortunately it’s suitable only for ...of converting the FTL syntax into an array of Transforms describing how the legacy translations should be migrated. This manner of defining migrations is only suitable to simple strings where a copy operation is sufficient. For more complex use-cases which require some additional logic in Python, you'll need to resort to the raw AST. ::: intl/l10n/docs/fluent_migrations.rst:192 (Diff revision 1) > + FTL.Message( > + id=FTL.Identifier("findbar-next"), > + attributes=[ > + FTL.Attribute( > + FTL.Identifier("tooltiptext"), > + COPY( Perhaps add the names of the keyword args (id, value) to make this snippet of the AST a little bit easier to understand? ::: intl/l10n/docs/fluent_migrations.rst:208 (Diff revision 1) > +:js:`findbar-next`. A message can have an array of attributes, each with an ID > +and a value: in this case there is only one attribute, with ID :js:`tooltiptext` > +and :js:`value` copied from the legacy string. > + > +Notice how both the ID of the message and the ID of the attribute are > +defined as an :python:`FTL.Identifier`, not simply as a string. I'm not sure if this sentence brings much value. Perhaps something more general would work better? Unfortunately, the best reference right now is https://github.com/projectfluent/fluent/tree/master/syntax. I'll need to update the Playground to use the refparser once Syntax 0.6 is published. Also note that we might be removing the Identifier nodes in the near future, so if we keep this sentence, we'll need to make sure we update it later on. See https://github.com/projectfluent/fluent/issues/144. ::: intl/l10n/docs/fluent_migrations.rst:213 (Diff revision 1) > +defined as an :python:`FTL.Identifier`, not simply as a string. > + > + > +.. tip:: > + > + It’s possible to concatenate arrays of Transforms manually defined, like in ...defined manually...
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #5) > ::: intl/l10n/docs/fluent_migrations.rst:120 > (Diff revision 1) > > + def migrate(ctx): > > + """Bug 1411707 - Migrate the findbar XBL binding to a Custom Element, part {index}.""" > > + > > + ctx.add_transforms( > > + "toolkit/toolkit/main-window/findbar.ftl", > > + "toolkit/toolkit/main-window/findbar.ftl", > > I wouldn't mind if this was just `toolkit/main-window/findbar.ftl`. > Documenting `bla/bla` seems to set a bad incentive. This is an actual migration (bug 1411707), and it matches the `/browser/browser`. > ::: intl/l10n/docs/fluent_migrations.rst:215 > (Diff revision 1) > > + > > +.. tip:: > > + > > + It’s possible to concatenate arrays of Transforms manually defined, like in > > + the last example, with those coming from :python:`transforms_from`, by using > > + the :python:`+` operator. > > Should we recommend this? > > I wonder if just doing multiple ctx.add_transforms is easier to digest? Added that there's an alternative, but personally I find it readable https://dxr.mozilla.org/mozilla-central/rev/f7fd9b08c0156be5b5cd99de5ed0ed0b98d93051/python/l10n/fluent_migrations/bug_1457021_js_subdialogs.py > > ::: intl/l10n/docs/fluent_migrations.rst:387 > (Diff revision 1) > > +.. code-block:: properties > > + > > + # LOCALIZATION NOTE (disableContainersOkButton): Semi-colon list of plural forms. > > + # See: http://developer.mozilla.org/en/docs/Localization_and_Plurals > > + # #S is the number of container tabs > > + disableContainersOkButton = Close #S Container Tab;Close #S Container Tabs > > #1 This is an actual migration, but I guess it makes no point using a broken string as an example. > ::: intl/l10n/docs/fluent_migrations.rst:524 > (Diff revision 1) > > +1. Install Fluent Migration > > +--------------------------- > > + > > +The first step is to install the `Fluent Migration`_ Python library. It’s > > +currently not available as a package, so the repository must be cloned locally > > +and installed manually, e.g. with :bash:`pip install -e .`. Yes. Happy to update the doc when that happens :) Addressing all other comments (hopefully).
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Staś Małolepszy :stas from comment #6) > ::: intl/l10n/docs/fluent_migrations.rst:22 > (Diff revision 1) > > +to localize hundreds of strings from scratch. > > + > > +`Fluent Migration`_ is a Python library designed to solve this specific problem: > > +it allows to migrate legacy translations from `.dtd` and `.properties` files, > > +not only moving strings and transforming them as needed to adapt to the `FTL` > > +syntax, but also replicating in VCS the history of each string. > > ...syntax. It also preserves some attribution history for each string in the > VCS. Used Pike's suggestion here. > ::: intl/l10n/docs/fluent_migrations.rst:45 > (Diff revision 1) > > + > > + - New Fluent strings land in `mozilla-central`, together with a migration > > + recipe. > > + - `gecko-strings-quarantine`_ – a unified repository including strings > > + for all shipping versions of Firefox, used as a buffer before exposing > > + strings to localizers – is updated to include the new strings. > > I'd try to rephrase this sentence in active voice: > > The new strings lands in gsq, a unified... I'm having a hard time switching this to active mode, mostly because all steps but the first require manual actions from l10n-drivers. > ::: intl/l10n/docs/fluent_migrations.rst:49 > (Diff revision 1) > > + for all shipping versions of Firefox, used as a buffer before exposing > > + strings to localizers – is updated to include the new strings. > > + - Migration recipes are run against all l10n repositories, migrating strings > > + from old to new files, and storing them in VCS. > > + - New en-US strings are pushed to the official `gecko-strings`_ repository > > + used by localization tools, and exposed to all localizers. > > Perhaps add an explicit step here which says that after this step, the > migrations end their life cycle? I know this is explained in the paragraph > below, but I think it would make sense to make it in to the last bullet > point on this list. This is partially true though, since we run these recipes for a few more time in the cycle. Adding some text though (also moved this section at the bottom of the doc) > > ::: intl/l10n/docs/fluent_migrations.rst:154 > (Diff revision 1) > > + > > +In this case there is only one Transform that migrates the string with ID > > +:js:`next.tooltip` from :bash:`toolkit/chrome/global/findbar.dtd`, and > > +inject it in the FTL fragment. The :python:`COPY` Transform allows to copy the > > +string from an existing file as is, while :python:`from_path` is used to avoid > > +repeating the same path multiple times, making the recipe less readable. Without > > Here again, I'd rather say "making the recipe more readable" because I > associate this phrase with "is used to avoid" :) Am I reading this wrong? Nope, you're correct. > > ::: intl/l10n/docs/fluent_migrations.rst:208 > (Diff revision 1) > > +:js:`findbar-next`. A message can have an array of attributes, each with an ID > > +and a value: in this case there is only one attribute, with ID :js:`tooltiptext` > > +and :js:`value` copied from the legacy string. > > + > > +Notice how both the ID of the message and the ID of the attribute are > > +defined as an :python:`FTL.Identifier`, not simply as a string. > > I'm not sure if this sentence brings much value. Perhaps something more > general would work better? Unfortunately, the best reference right now is > https://github.com/projectfluent/fluent/tree/master/syntax. I'll need to > update the Playground to use the refparser once Syntax 0.6 is published. > > Also note that we might be removing the Identifier nodes in the near future, > so if we keep this sentence, we'll need to make sure we update it later on. > See https://github.com/projectfluent/fluent/issues/144. Personally I don't think it's intuitive that this can't be a simple string, that's why I'm calling it out. Addressed the comments, not sure if I didn't break the syntax (don't have mach on this machine, will need to check tomorrow).
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review257030 This looks good to me, I resolved the issues that I think are resolved. I couldn't do that for stas' comments, and there might be one open still about using `id` and `value` for one of the Attribute nodes? Commonly the patch author goes in and resolves the issues in the updated patch, and then the reviewer just verifies that, btw.
Attachment #8984195 -
Flags: review?(l10n) → review+
Assignee | ||
Comment 13•6 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #12) > Commonly the patch author goes in and resolves the issues in the updated > patch, and then the reviewer just verifies that, btw. Sorry about that, I normally close them before landing (and I've seen it done that way in other cases), but I see how it makes more sense in case of re-review.
Reporter | ||
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review257036 ::: intl/l10n/docs/fluent_migrations.rst:273 (Diff revision 4) > + > + value=REPLACE( > + "browser/chrome/browser/preferences/preferences.properties", > + "searchResults.sorryMessageWin", > + { > + "%S": FTL.TextElement("<span data-l10n-name="query"></span>") This example will not work because of the use of `"` mark both to encode a string and within it. You may have to switch to `'` or escape them. ::: intl/l10n/docs/fluent_migrations.rst:296 (Diff revision 4) > + > +Concatenating Strings > +--------------------- > + > +It’s quite common to concatenate multiple strings coming from `DTD` and > +`properties`, for example to create sentences with HTML markup. It’s possible to I think there's an opportunity here to reference Fluent best practies on why you should concatenate here and not at runtime - https://firefox-source-docs.mozilla.org/intl/l10n/l10n/fluent_tutorial.html#social-contract
Attachment #8984195 -
Flags: review?(gandalf) → review+
Reporter | ||
Comment 15•6 years ago
|
||
Looks great Flod! Thanks so much for writing this down :)
Assignee | ||
Comment 16•6 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #14) > Comment on attachment 8984195 [details] > Bug 1467183 - Add documentation for Fluent migrations, > > https://reviewboard.mozilla.org/r/249996/#review257036 > > ::: intl/l10n/docs/fluent_migrations.rst:273 > (Diff revision 4) > > + > > + value=REPLACE( > > + "browser/chrome/browser/preferences/preferences.properties", > > + "searchResults.sorryMessageWin", > > + { > > + "%S": FTL.TextElement("<span data-l10n-name="query"></span>") > > This example will not work because of the use of `"` mark both to encode a > string and within it. You may have to switch to `'` or escape them. Uh, you're absolutely right. I replaced quotes for consistency, and didn't think about this. Fixing it. > ::: intl/l10n/docs/fluent_migrations.rst:296 > (Diff revision 4) > > + > > +Concatenating Strings > > +--------------------- > > + > > +It’s quite common to concatenate multiple strings coming from `DTD` and > > +`properties`, for example to create sentences with HTML markup. It’s possible to > > I think there's an opportunity here to reference Fluent best practies on why > you should concatenate here and not at runtime - > https://firefox-source-docs.mozilla.org/intl/l10n/l10n/fluent_tutorial. > html#social-contract Good point. I need to figure out how to link to a section of another doc, for now linking to the doc itself. Also need to figure out that the syntax works, as soon as I have the build system available.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review257052 ::: intl/l10n/docs/fluent_migrations.rst:300 (Diff revisions 4 - 5) > It’s quite common to concatenate multiple strings coming from `DTD` and > `properties`, for example to create sentences with HTML markup. It’s possible to > -concatenate strings and text element in a migration recipe using the > -:python:`CONCAT` Transform. > +concatenate strings and text elements in a migration recipe using the > +:python:`CONCAT` Transform. This allows to generate a single Fluent message from > +these fragments, avoiding run-time transformations as prescribed by > +:doc:`Fluent’s social contract <fluent_tutorial>`. This should link to the anchor, and make sure it works when building docs locally.
Comment hidden (mozreview-request) |
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review258190 Amazing work, flod, putting all this know-how into text! In places where I suggest to add a sentence, my suggestions are only that, suggestions. Feel free to rephrase as you see fit. ::: intl/l10n/docs/fluent_migrations.rst:91 (Diff revision 6) > + > + findbar-next = > + .tooltiptext = Find the next occurrence of the phrase > + > + > +This is how the migration recipe looks like: Grammar nit: I've been tought that it's either "How does it look" or "What does it look like". There also a subtle difference in meaning: the first one is asking for an adverb: it looks good/bad while the second one is asking for a description. ::: intl/l10n/docs/fluent_migrations.rst:142 (Diff revision 6) > + - An array of Transforms. Transforms are AST nodes which describe how legacy > + translations should be migrated. > + > +In this case there is only one Transform that migrates the string with ID > +:js:`next.tooltip` from :bash:`toolkit/chrome/global/findbar.dtd`, and > +inject it in the FTL fragment. The :python:`COPY` Transform allows to copy the Nit: injects ::: intl/l10n/docs/fluent_migrations.rst:208 (Diff revision 6) > +.. tip:: > + > + It’s possible to concatenate arrays of Transforms defined manually, like in > + the last example, with those coming from :python:`transforms_from`, by using > + the :python:`+` operator. Alternatively, it’s possible to use multiple > + :python:`add_transforms`. It might be a good idea to mention here that the order of transforms doesn't matter. The reference file is used for ordering. ::: intl/l10n/docs/fluent_migrations.rst:278 (Diff revision 6) > + "%S": FTL.TextElement('<span data-l10n-name="query"></span>') > + } > + ) > + > + > +.. note:: I'd add here at the beginning of the note: EXTERNAL_ARGUMENT and MESSAGE_REFERENCE are helper transforms which can be used to save keystrokes in common cases using the raw AST is too verbose. ::: intl/l10n/docs/fluent_migrations.rst:300 (Diff revision 6) > +It’s quite common to concatenate multiple strings coming from `DTD` and > +`properties`, for example to create sentences with HTML markup. It’s possible to > +concatenate strings and text elements in a migration recipe using the > +:python:`CONCAT` Transform. This allows to generate a single Fluent message from > +these fragments, avoiding run-time transformations as prescribed by > +:ref:`Fluent’s social contract <fluent-tutorial-social-contract>`. I'd add here: Note that in the case of simple migrations using `transforms_from`, the concatenation is carried out implicitly by the fact of using the Fluent syntax interleaved with COPY() transform calls to define the migration recipe. ::: intl/l10n/docs/fluent_migrations.rst:335 (Diff revision 6) > + value=REPLACE( > + "browser/chrome/browser/preferences/preferences.properties", > + "searchResults.needHelp", > + { > + "%S": CONCAT( > + FTL.TextElement("<a data-l10n-name="url">"), Double quotes around "url" need to be escaped here. ::: intl/l10n/docs/fluent_migrations.rst:421 (Diff revision 6) > + > + > +The `PLURALS` Transform will take care of creating the correct number of plural > +categories for each language. Notice how `#1` is replaced for each of these > +variants with :js:`{ $tabCount }`, using :python:`REPLACE_IN_TEXT` and > +:python:`EXTERNAL_ARGUMENT("tabCount")`. I'd add here: We can't use REPLACE here because it takes a file path and sa string id as arguments, whereas here we operate on regular text. We perform the replacement on each plural form of the original string separated by the semicolon. ::: intl/l10n/docs/fluent_migrations.rst:457 (Diff revision 6) > + FTL.Message( > + id=FTL.Identifier("use-current-pages"), > + attributes=[ > + FTL.Attribute( > + FTL.Identifier("label"), > + FTL.Pattern( Please use "id" and "value" as argument names here. ::: intl/l10n/docs/fluent_migrations.rst:464 (Diff revision 6) > + FTL.Placeable( > + expression=FTL.SelectExpression( > + expression=EXTERNAL_ARGUMENT("tabCount"), > + variants=[ > + FTL.Variant( > + key=FTL.VariantName("1"), I think this should be an FTL.Number(1)? ::: intl/l10n/docs/fluent_migrations.rst:487 (Diff revision 6) > + ] > + ) > + ), > + FTL.Attribute( > + FTL.Identifier("accesskey"), > + COPY( Explicit "id" and "value" here as well, please. ::: intl/l10n/docs/fluent_migrations.rst:499 (Diff revision 6) > + > + > +This Transform uses several concepts already described in this document. Notable > +new elements are: > + > + - The fact that the `label` attribute is defined as a :python:`Pattern`. I'd add after this sentence: It's because in this example we're creating a new value from scratch and migrating existing translations as its variants. Patterns are one of Fluent's value types. ::: intl/l10n/docs/fluent_migrations.rst:531 (Diff revision 6) > +2. Clone gecko-strings > +---------------------- > + > +Migration recipes work against localization repositories, which means it’s not > +possible to test them directly against `mozilla-central`, unless the *source* > +path in :python:`ctx.add_transforms` is temporarily tweaked to match ...unless the source path (the second argument)... ::: intl/l10n/docs/fluent_migrations.rst:535 (Diff revision 6) > +possible to test them directly against `mozilla-central`, unless the *source* > +path in :python:`ctx.add_transforms` is temporarily tweaked to match > +`mozilla-central` paths. > + > +To test the actual recipe that will land in the patch, it’s necessary to clone > +the `gecko-strings`_ repository on the system twice, in two separate folders. ...One will simulate the reference en-US repository after the patch has landed, and the other will simulate a target localization... ::: intl/l10n/docs/fluent_migrations.rst:574 (Diff revision 6) > + name_of_the_recipe > + > + > +:bash:`PYTHONPATH` is updated to include the folder storing migration recipes. > +The name of the recipe needs to be specified without the :bash:`.py` extension, > +since it’s imported as a module. Maybe just recommend to `cd PATH/TO/recipes` rather than setting the PYTHONPATH? ::: intl/l10n/docs/fluent_migrations.rst:676 (Diff revision 6) > + > +Don’t hesitate to reach out to the l10n-drivers for feedback, help to test or > +write the migration recipes: > + > + - Francesco Lodolo (:flod) > + - Stas Malolepszy (:stas) Any chance I could get my funny Polish letters here? :) Staś Małolepszy
Attachment #8984195 -
Flags: review?(stas) → review+
Assignee | ||
Comment 21•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8984195 [details] Bug 1467183 - Add documentation for Fluent migrations https://reviewboard.mozilla.org/r/249996/#review258190 > I think this should be an FTL.Number(1)? This is how the original recipe was written https://github.com/flodolo/fluent-migrations/blob/master/recipes/fx60/bug_1435912_preferences_general_xul.py#L219 I assume they both produce the same result, but isn't more logic to use `FTL.VariantName`, since we're build a variant?
Comment hidden (mozreview-request) |
Comment 23•6 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #21) > I assume they both produce the same result, but isn't more logic to use > `FTL.VariantName`, since we're build a variant? Variant keys are of type VariantKey, which can be either a VariantName (when it's text) or a NumberExpression (renamed to NumberLiteral in Syntax 0.6; not a "Number" like I mistakenly wrote above). The distinction is an important one in the runtime: it's how the resolver can know to treat a key as a number and apply special logic to it. As you say, for serialization this doesn't really matter, but semantically we should use NumberExpression(1) here.
Comment hidden (mozreview-request) |
Comment 25•6 years ago
|
||
Pushed by francesco.lodolo@mozillaitalia.org: https://hg.mozilla.org/integration/autoland/rev/97a8832286af Add documentation for Fluent migrations, r=gandalf,Pike,stas
Comment 26•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/97a8832286af
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•