Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9209 closed enhancement (fixed)

[patch] add more lists to class Ticket, delete unneeded whitespace

Reported by: hoff.st@… Owned by: hasienda <hoff.st@…>
Priority: normal Milestone: 1.0
Component: ticket system Version: 0.12dev
Severity: trivial Keywords: cleanup deduplication
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

on code review for another enhancement I recognized lists being constructed multiple times in trac/ticket/model.py, probably grown so for historical reasons

first attached patch adds to class Ticket

  • self.std_fields
  • self.custom_fields

changes occurrences of *_fields to self.*_fields accordingly and deletes all obsoleted statements for re-construction of said lists. I did it, because I plan to use ticket.custom_fields in trac/ticket/web_ui.py too.

second attached patch just deletes some occurrences of obsolete, unneeded whitespace, if you care

Attachments (6)

add-ticket-class-var_to_ticket-model-py.patch (4.8 KB ) - added by hoff.st@… 11 years ago.
patch for trac/ticket/model.py: 14 insertions, 28 deletions
cleanup_ticket-model-py.patch (8.2 KB ) - added by hoff.st@… 11 years ago.
patch for trac/ticket/model.py: 28 insertions, 32 deletions of whitespace
add-ticket-class-var_to_ticket-model-py.2.patch (4.1 KB ) - added by hoff.st@… 11 years ago.
patch for trac/ticket/model.py - updated version: 17 insertions, 20 deletions
cleanup_ticket-model-py.2.patch (8.2 KB ) - added by hoff.st@… 11 years ago.
patch for trac/ticket/model.py - updated version to adjust for changes in other patch: 28 insertions, 32 deletions of whitespace
add-ticket-class-var_to_ticket-model-py.3.patch (4.5 KB ) - added by hoff.st@… 11 years ago.
patch for trac/ticket/model.py - 2nd update: 26 insertions, 25 deletions
cleanup_ticket-model-py.3.patch (8.2 KB ) - added by hoff.st@… 11 years ago.
patch for trac/ticket/model.py - 2nd update to adjust for changes in other patch: 28 insertions, 32 deletions of whitespace

Download all attachments as: .zip

Change History (14)

by hoff.st@…, 11 years ago

patch for trac/ticket/model.py: 14 insertions, 28 deletions

by hoff.st@…, 11 years ago

patch for trac/ticket/model.py: 28 insertions, 32 deletions of whitespace

comment:1 by hoff.st@…, 11 years ago

seems the first patch needs some fixing, Exception raised when testing, suggests that 'resolution' key in std_fields is not present in dict values when creating new ticket, so seems like I've found one of the reasons for the re-assembling of this list

wouldn't it be best to just set the missing key like

values['resolution'] = ''

than to work around another way?

comment:2 by hoff.st@…, 11 years ago

undoing deletion of private list std_fields in do_insert() inside (Ticket.)insert() fixes this, going on with some more test, to be sure

by hoff.st@…, 11 years ago

patch for trac/ticket/model.py - updated version: 17 insertions, 20 deletions

by hoff.st@…, 11 years ago

patch for trac/ticket/model.py - updated version to adjust for changes in other patch: 28 insertions, 32 deletions of whitespace

comment:3 by hoff.st@…, 11 years ago

Sorry for the delay. After more testing I recommend going on with fixed version added above, so look at both *.2.patch now. For more comments on this I recommend reviewing my corresponding notes in http://bitbucket.org/hasienda/custom-time/src/tip/series, since this is part of a bigger effort. Thanks for taking care.

comment:4 by Christian Boos, 11 years ago

Both patches look fine.

List comprehensions are good, but in this specific case as you need 3 of them (self.std_fields = [f['name'] for f in self.fields ...) maybe it would be a bit more efficient and clearer to expand those 3 in one single for loop iteration.

comment:5 by Christian Boos, 11 years ago

Milestone: 0.13

by hoff.st@…, 11 years ago

patch for trac/ticket/model.py - 2nd update: 26 insertions, 25 deletions

by hoff.st@…, 11 years ago

patch for trac/ticket/model.py - 2nd update to adjust for changes in other patch: 28 insertions, 32 deletions of whitespace

comment:6 by hoff.st@…, 11 years ago

Actually, the logic you requested was already there some lines below. I merged best of both loops to shorten some statements a little and make variable fname obsolete. This is version 3 of both patches now. Thanks for the encouragement to improve this.

comment:7 by Remy Blank, 11 years ago

Resolution: fixed
Status: newclosed

Applied add-ticket-class-var_to_ticket-model-py.3.patch to trunk in [10045] and cleanup_ticket-model-py.3.patch to 0.12-stable in [10046] (to facilitate merging).

comment:8 by Remy Blank, 11 years ago

Owner: set to hasienda <hoff.st@…>

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain hasienda <hoff.st@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from hasienda <hoff.st@…> to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.