Closed Bug 1604663 Opened 2 years ago Closed 4 days ago

Include cZ in merge comparison

Categories

(SeaMonkey :: Chat, task)

task
Not set
normal

Tracking

(seamonkey2.53+ fixed, seamonkey2.57esr? affected)

RESOLVED FIXED
seamonkey 2.88
Tracking Status
seamonkey2.53 + fixed
seamonkey2.57esr ? affected

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

Details

(Whiteboard: SM2.53.8)

Attachments

(2 files, 8 obsolete files)

At the moment cZ is not included when you run a merge comparison.

Attached patch patch for SM's l10n.toml (obsolete) — Splinter Review

Cannot see a way of excluding if cZ is not being built

Attachment #9116572 - Flags: review?(frgrahl)
Attached patch patch to create l10n.toml for cZ (obsolete) — Splinter Review
Attachment #9116574 - Flags: review?(frgrahl)

Correct patch attached

Attachment #9116574 - Attachment is obsolete: true
Attachment #9116574 - Flags: review?(frgrahl)
Attachment #9116596 - Flags: review?(frgrahl)

Ignore MOZ_LANGPACK_CONTRIBUTORS

Attachment #9116596 - Attachment is obsolete: true
Attachment #9116596 - Flags: review?(frgrahl)
Attachment #9116605 - Flags: review?(frgrahl)
Comment on attachment 9116572 [details] [diff] [review]
patch for SM's l10n.toml

Because this doesn't work when irc is disabled we should do it after integrating cZ into our tree and product.
Attachment #9116572 - Flags: review?(frgrahl) → feedback+
Comment on attachment 9116605 [details] [diff] [review]
patch to create l10n.toml for cZ v1.1

f+ for the moment 

ca is dead. 

We should wait till we intergate cZ into our product. Depending on how we do it this is not needed then. In any case locales would need to be moved to suite/extensions/irc/locales/... and depending on version mozilla as top source dir taken into account.
Attachment #9116605 - Flags: review?(frgrahl) → feedback+
Component: Build Config → Chat
Duplicate of this bug: 1312353
Attached patch 1604663-irc-merge-2538.patch (obsolete) — Splinter Review

[Approval Request Comment]
Regression caused by (bug #): n/a
User impact if declined: none
Testing completed (on m-c, etc.): 2.53.8
Risk to taking this patch (and alternatives if risky): low
String changes made by this patch: none

As we have now forked the cZ code, this should be achievable.
A few notes:

  • I have tested this patch against both existing locales (de and en-GB) and one that doesn't localise cZ (el).
  • There may still be unneeded code in the moz.build/Makefile.in files, if you spot any say.
  • In the l10n.toml / all-locales for our cZ we could remove ca and si locales as SM does not build for them.
  • This patch requires for each locale that extensions/irc is copied/moved to suite/extensions/irc
  • If / when this lands on l10n-central we need to arrange for the entities in each locale to be copied from extensions/irc to suite/extensions/irc and check that we are only exposing the entities in suite/extensions/irc to the locales that are in our cZ's l10n.toml rather than to all SM's locales.
Attachment #9116572 - Attachment is obsolete: true
Attachment #9116605 - Attachment is obsolete: true
Attachment #9217587 - Flags: review?(frgrahl)
Attachment #9217587 - Flags: approval-comm-release?
Attachment #9217587 - Flags: approval-comm-esr60?

Additional note:

  • Next version of this patch should remove the xpi directory under suite/extensions/irc/ (but that could be a separate bug/patch if preferred).

Changes needed for toolkit's l10n.mk file

Attachment #9217847 - Flags: review?(frgrahl)
Attachment #9217847 - Flags: approval-comm-release?
Attachment #9217847 - Flags: approval-comm-esr60?

Patch updated to remove unneeded xpi/ files

Attachment #9217587 - Attachment is obsolete: true
Attachment #9217587 - Flags: review?(frgrahl)
Attachment #9217587 - Flags: approval-comm-release?
Attachment #9217587 - Flags: approval-comm-esr60?
Attachment #9217851 - Flags: review?(frgrahl)
Attachment #9217851 - Flags: approval-comm-release?
Attachment #9217851 - Flags: approval-comm-esr60?
Depends on: 1707420

Comment on attachment 9217851 [details] [diff] [review]
1694760-mailservices-base-v1_2-2538.patch

Wrong patch

Attachment #9217851 - Attachment is obsolete: true
Attachment #9217851 - Flags: review?(frgrahl)
Attachment #9217851 - Flags: approval-comm-release?
Attachment #9217851 - Flags: approval-comm-esr60?

Updated version, tested both unpackaged and packaged localised version.

Attachment #9218139 - Flags: review?(frgrahl)
Attachment #9218139 - Flags: approval-comm-release?
Attachment #9218139 - Flags: approval-comm-esr60?
Attached patch 1604663-irc-installer-2538.patch (obsolete) — Splinter Review

Requesting feedback as needs to be tested on non-linux platforms. Not sure what is needed in removed-files.in

Attachment #9218140 - Flags: feedback?(frgrahl)
Attachment #9217847 - Flags: review?(frgrahl)
Attachment #9217847 - Flags: approval-comm-release?
Attachment #9217847 - Flags: approval-comm-esr60?
Attachment #9218139 - Flags: review?(frgrahl)
Attachment #9218139 - Flags: approval-comm-release?
Attachment #9218139 - Flags: approval-comm-esr60?
Attachment #9218140 - Flags: feedback?(frgrahl)
Depends on: 1707458

Changes since last patch:

  • Combined the merge and installer patches
  • Remove xpi changes as they are now in Bug 1707458
  • Found the fixes for packaging extension l10n in app langpacks
Attachment #9218139 - Attachment is obsolete: true
Attachment #9218140 - Attachment is obsolete: true
Attachment #9218759 - Flags: review?(frgrahl)
Attachment #9218759 - Flags: approval-comm-release?
Attachment #9217847 - Flags: review?(frgrahl)
Attachment #9217847 - Flags: approval-comm-release?
See Also: → 1707459
Whiteboard: SM2.53.8

Comment on attachment 9217847 [details] [diff] [review]
1604663-irc-l10n-2538.patch

LGTM

Attachment #9217847 - Flags: review?(frgrahl)
Attachment #9217847 - Flags: review+
Attachment #9217847 - Flags: approval-comm-release?
Attachment #9217847 - Flags: approval-comm-release+

Comment on attachment 9218759 [details] [diff] [review]
1604663-irc-merge-v1_3-2538.patch

LGTM

+DEFINES['MOZ_APP_VERSION'] = CONFIG['MOZ_APP_VERSION']
+DEFINES['MOZ_APP_MAXVERSION'] = CONFIG['MOZ_APP_MAXVERSION']

NIT:
I think we should remove MOZ_APP_MAXVERSION and just set the max version to MOZ_APP_VERSION in the rdf. cZ is now bundled and should always match.

Would it make sense for this bug to be included in 2.57 and c-c too and just turn bug 1707459 into an incremental bug for the move and conversion?. Both should be checked in at the same time. Reason is that we then can just add bug 1707459 to 2.53 if desired later.

Attachment #9218759 - Flags: review?(frgrahl)
Attachment #9218759 - Flags: review+
Attachment #9218759 - Flags: approval-comm-release?
Attachment #9218759 - Flags: approval-comm-release+

Comment on attachment 9217847 [details] [diff] [review]
1604663-irc-l10n-2538.patch

Also for 2.57

Attachment #9217847 - Flags: approval-comm-esr60?

Comment on attachment 9218759 [details] [diff] [review]
1604663-irc-merge-v1_3-2538.patch

Also for 2.57

Attachment #9218759 - Flags: approval-comm-esr60?
Blocks: 1707459
See Also: 1707459

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/de28453479ac
Include cZ in merge comparison. r=frg

Status: ASSIGNED → RESOLVED
Closed: 4 days ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey 2.88

Comment on attachment 9218759 [details] [diff] [review]
1604663-irc-merge-v1_3-2538.patch

In unofficial queue as 1604663-irc-merge-v1_3a-257.patch

Attachment #9218759 - Flags: approval-comm-esr60? → approval-comm-esr60+

Comment on attachment 9217847 [details] [diff] [review]
1604663-irc-l10n-2538.patch

in unoffical queue as 1604663-irc-l10n-257.patch

Attachment #9217847 - Flags: approval-comm-esr60? → approval-comm-esr60+
You need to log in before you can comment on or make changes to this bug.