Closed Bug 1604663 Opened 5 years ago Closed 3 years ago

Include cZ in merge comparison

Categories

(SeaMonkey :: Chat, task)

Product:
SeaMonkey
Component:
Chat
Type:
task
Priority:
Not set
Severity:
normal

Tracking

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

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

People

(Reporter: iannbugzilla, Assigned: iannbugzilla)

References

Details

(Whiteboard: SM2.53.8)

Attachments

(2 files, 8 obsolete files)

1604663-irc-l10n-2538.patch
(deleted), patch
frg
: review+
frg
: approval-comm-release+
frg
: approval-comm-esr60+
Details | Diff | Splinter Review
1604663-irc-merge-v1_3-2538.patch
(deleted), patch
frg
: review+
frg
: approval-comm-release+
frg
: approval-comm-esr60+
Details | Diff | Splinter Review

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

Attached patch patch for SM's l10n.toml (obsolete) (deleted) — — 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) (deleted) — — Splinter Review
Attachment #9116574 - Flags: review?(frgrahl)
Attached patch correct patch to create l10n.toml for cZ (obsolete) (deleted) — — Splinter Review

Correct patch attached

Attachment #9116574 - Attachment is obsolete: true
Attachment #9116574 - Flags: review?(frgrahl)
Attachment #9116596 - Flags: review?(frgrahl)
Attached patch patch to create l10n.toml for cZ v1.1 (obsolete) (deleted) — — Splinter Review

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
Attached patch 1604663-irc-merge-2538.patch (obsolete) (deleted) — — 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).
Attached patch 1604663-irc-l10n-2538.patch (deleted) — — Splinter Review

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?
Attached patch 1694760-mailservices-base-v1_2-2538.patch (obsolete) (deleted) — — Splinter Review

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?
Attached patch 1604663-irc-merge-v1_2-2538.patch (obsolete) (deleted) — — Splinter Review

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) (deleted) — — 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?

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: https://bugzilla.mozilla.org/show_bug.cgi?id=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: 3 years ago
Resolution: --- → FIXED
status-seamonkey2.57esr: unaffected → affected
tracking-seamonkey2.57esr: --- → ?
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.

Attachment

General

Creator:
Created:
Updated:
Size: