-
Notifications
You must be signed in to change notification settings - Fork 22
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
Update firefox faq about uplifts #293
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@@ -50,7 +50,7 @@ Typically once or twice a week, the quarantined content (in the `update` branch) | |||
|
|||
While Nightly is always open for new strings, Beta and Release are string frozen, meaning that patches with new strings are normally not allowed to land. | |||
|
|||
Uplifts need to be evaluated case by case, but in general they should be avoided in the context of a 4-weeks release cycle. Possible alternatives are uplifting ad-hoc patches without string changes, or exposing the feature/change only to English users. | |||
Uplifts need to be evaluated case by case, but in general they should be avoided in the context of a 4-weeks release cycle. Possible alternatives are uplifting ad-hoc patches without string changes, or exposing the feature/change only to English users. In the event an uplift request is made and approved, an updated [`l10n-changesets.json`](https://searchfox.org/mozilla-central/source/browser/locales/l10n-changesets.json) with the revision set to an appropriate commit hash needs to be included. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit too vague.
First issue: the update doesn't "need" to be included. It should be included if we landed the strings and gave people time to translate them. If it's an urgent uplift, and the strings are still stuck in quarantine, no point in uplifting an updated l10n-changesets.json
.
What does "include" mean? Include it in the patch for uplift, ask release managers to include the update?
What is an "update"? From where do you take the file to uplift, how does it look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully this commit addresses your feedback.
What does "include" mean? Include it in the patch for uplift, ask release managers to include the update?
I'm not sure if there's a better approach to what I added, but I think having the changes to l10n-changesets.json
in the uplift patch would allow us an opportunity to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an option if they need to create a separate patch for uplift, but typically the patch is uplifted as is. We should mention both options: ad-hoc patch including the updated file, or ask relman to land a follow-up change.
We also typically take the l10n-changesets file from mozilla-beta, even if it's not the latest, since it means it's gone through the build system at least once.
We should also link to https://mozilla-l10n.github.io/documentation/products/firefox_desktop/build_system.html#overview
Fixes #288