Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docs for adding a new l10n into product-details #89

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MihaiTabara
Copy link
Contributor

@MihaiTabara MihaiTabara commented May 17, 2021

@escapewindow @bhearsum Does this sound good to you? @flodolo and myself were debating earlier today whether this is the right way to go or do we need to wait another beta build to happen in order to populate the values?

Facts I understand so far, can one of you please confirm I'm correct?

  • I'm assuming everyone else is consuming from S3 which is the single-source-of-truth (e.g. bedrock reading the firefox_primary_builds.json file to populate https://www.mozilla.org/en-US/firefox/all/#product-desktop-nightly or any other downstream consumer, etc)
  • looking at the CI in the product-details, each commit in production is publishing the repo artifacts to the S3 bucket
  • in order to deploy, we can either manually review + merge PRs into production (1) or let the https://github.com/release-services-robot force-push (2) do it.

For (1) - that means that we're adjusting the files directly in this repo that we publish and that becomes the new source-of-truth
For (2) - the changes are being triggered by the rebuild function which basically meshes together the existing product-details repo, hgmo + Ship-it DB and creates a diff, if there's any change found. Note that the push happens automatically toproduction rather than via pull-requests.

So to sum-up, there's two ways of updating product-details:

  • either via PRs directly in this repo
  • by consuming a Pulse event message that triggers the rebuild function

Who's publishing the Pulse message then?

  • by clicking the Rebuild Product-details button in the Ship-it UI which, if I read right the JavaScript code in here hits the /product-details endpoint via javascript here which is described here as the Pulse message publisher in the API here
  • there's Ship-it logic that triggers the same Pulse publishing event each time we update a release status in here e.g. mark a beta release as shipped

So each time we push the button in the UI or a release is marked as shipped, there's a Pulse message being published.
The last dumb question that lingers in my head is: am I to assume there's a worker that monitors (basically that constantly runs this)to listen to the Pulse messaging queue in our GCP workload? Looking at the git history of this repo, I'm assuming there is, but I just want to make sure I'm not missing a piece of the puzzle.

If I'm right with above, this explains why deploying this to production needs to UI-button-click, since the logic in here would return no changes, hence a no-op to publish afterwards. If so, I need to amend the docs in this PR.

@MihaiTabara MihaiTabara requested a review from bhearsum May 17, 2021 12:38
- double-check that the PR is listing another locale in the ``public/1.0/languages.json`` file. See samples PRs like `this`_ or `this <https://github.com/mozilla-releng/product-details/pull/9>`__ from the past
- review and merge the PR, either directly into ``production`` or via ``main`` and then follow-up with a push to ``production``
- wait for the Taskcluster CI to run successfully. Behind the scenes, the CI is cloning the repo resources and pushes them to a S3 bucket
- once the CI is green, trigger the Product-Details rebuild via Ship-it: |product-details-rebuild|
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: I may be wrong here - pending a conclusion in the PR conversation.

.. _Product details: https://product-details.mozilla.org/1.0/
.. _here: https://wiki.mozilla.org/Release_Management/Product_details
.. _this: https://github.com/mozilla-releng/product-details/pull/10
.. |product-details-rebuild| image:: /procedures/release-duty/faq/media/product-details-rebuild.png
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: this URL will fix itself once we merge as the abspath will become valid in the main repo.

opened against Product-details. TODO for releaseduty:


- double-check that the PR is listing another locale in the ``public/1.0/languages.json`` file. See samples PRs like `this`_ or `this <https://github.com/mozilla-releng/product-details/pull/9>`__ from the past
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean another locale = a new locale?



- double-check that the PR is listing another locale in the ``public/1.0/languages.json`` file. See samples PRs like `this`_ or `this <https://github.com/mozilla-releng/product-details/pull/9>`__ from the past
- review and merge the PR, either directly into ``production`` or via ``main`` and then follow-up with a push to ``production``
Copy link

@flodolo flodolo May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is always done against production, because the JSON file doesn't exist in main

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the main branch used for anything? Should we make production the default branch and delete the main branch?

ship a newly-localized Firefox in a different language, RelEng gets involved.
In order for that to happen, the l10n team adds the corresponding builds as a
pre-requisite. Then, they likely ping releaseduty: PR like `this`_ or
`this <https://github.com/mozilla-releng/product-details/pull/9>`__ are
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on our conversation elsewhere, and my reading of the code, I'm pretty sure that PRs against product-details are a bad idea. As far as I can tell, Ship It will rebuild product details based on the locales it finds in hg.mozilla.org (specifically, browser/locales/l10n-changesets.json).

So, a PR will work, but it will get overridden the next time we ship something, or trigger a manual rebuild of product details in Ship It.

Assuming this is all correct, this means that the correct procedure is:

  1. Land an hg.mozilla.org change to add the locale
  2. Trigger a manual rebuild of product details in Ship It

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that PRs against product-details are a bad idea

Sorry, I don't follow. product-details is the only place where the language name is defined, and that's the only change that those PRs contain (it doesn't touch the actual build information).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears the PRs @MihaiTabara are pointing to change the languages.json file, which contains human-readable descriptions of the locales. It looks like the release services robot PRs don't update languages.json, nor does l10n-changesets.json contain that information. We may need both, until or unless we add the additional human-readable information into l10n-changesets.json or similar, and update the bot to also update languages.json.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I stand corrected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants