Adding bugzilla messaging system in Quips page

RESOLVED FIXED in Bugzilla 5.0

Status

()

Bugzilla
Query/Bug List
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sjoshi, Assigned: sjoshi)

Tracking

unspecified
Bugzilla 5.0
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.68 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Created attachment 806839 [details] [diff] [review]
patch.diff

Bugzilla Messaging system is missing from Quips pages.
(Assignee)

Updated

5 years ago
Attachment #806839 - Flags: review?(simon)
(Assignee)

Updated

5 years ago
Assignee: general → joshi_sunil

Updated

5 years ago
Severity: normal → enhancement
Target Milestone: --- → Bugzilla 5.0

Updated

5 years ago
Component: Bugzilla-General → Query/Bug List

Comment 1

5 years ago
Comment on attachment 806839 [details] [diff] [review]
patch.diff

+    [% approved.size %] quips approved and [% unapproved.size %] quips unapproved.

Nit: "quip[% approved.size == 1 ? '' : 's' %]" and likewise for unapproved.
Attachment #806839 - Flags: review?(simon) → review-

Comment 2

5 years ago
(In reply to Simon Green from comment #1)
> Nit: "quip[% approved.size == 1 ? '' : 's' %]" and likewise for unapproved.

If you r- the patch based on this, then this is not a nit. :)

Sunil simply moved the strings into another file. They were already written like this. And personally, I prefer the current format than the unreadable one you suggested (it's hack which doesn't work on all languages).

Comment 3

5 years ago
(In reply to Frédéric Buclin from comment #2)
> If you r- the patch based on this, then this is not a nit. :)

Yes it can. A nit is "a minor shortcoming". This is a minor thing, but I believe a new patch should be supplied.
 
> Sunil simply moved the strings into another file. They were already written
> like this.

I'm going to be hypocritical (because some times I copy and paste bad code and intentionally don't fix it), but it is better to fix these things while we are moving them.

> And personally, I prefer the current format than the unreadable
> one you suggested (it's hack which doesn't work on all languages).

I disagree. It's good practice to use good English when displaying messages to users. It shows some professionalism in your coding practise. Other languages use their own .html.tmpl files.

  -- simon

Updated

5 years ago
Status: NEW → ASSIGNED

Updated

5 years ago
Blocks: 952793

Comment 4

5 years ago
Created attachment 8351013 [details] [diff] [review]
patch, v2

Nit fixed. r=LpSolit
Attachment #806839 - Attachment is obsolete: true
Attachment #8351013 - Flags: review+

Updated

5 years ago
Flags: approval?
Since we're not using gettext, I agree with handling the language thing this way.  If we were actually using gettext it would be a different story.
Flags: approval? → approval+

Comment 6

5 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified quips.cgi
modified template/en/default/global/messages.html.tmpl
modified template/en/default/list/quips.html.tmpl
Committed revision 8838.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.