Ticket #9209 (closed enhancement: fixed)
Opened 22 months ago
Last modified 17 months ago
[patch] add more lists to class Ticket, delete unneeded whitespace
| Reported by: | hoff.st@… | Owned by: | hasienda <hoff.st@…> |
|---|---|---|---|
| Priority: | normal | Milestone: | 0.13 |
| Component: | ticket system | Version: | 0.12dev |
| Severity: | trivial | Keywords: | cleanup deduplication |
| Cc: | |||
| Release Notes: | |||
| API 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
Change History
Changed 22 months ago by hoff.st@…
- Attachment add-ticket-class-var_to_ticket-model-py.patch added
Changed 22 months ago by hoff.st@…
- Attachment cleanup_ticket-model-py.patch added
patch for trac/ticket/model.py: 28 insertions, 32 deletions of whitespace
comment:1 Changed 22 months ago by hoff.st@…
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 Changed 22 months ago by hoff.st@…
undoing deletion of private list std_fields in do_insert() inside (Ticket.)insert() fixes this, going on with some more test, to be sure
Changed 22 months ago by hoff.st@…
- Attachment add-ticket-class-var_to_ticket-model-py.2.patch added
patch for trac/ticket/model.py - updated version: 17 insertions, 20 deletions
Changed 22 months ago by hoff.st@…
- Attachment cleanup_ticket-model-py.2.patch added
patch for trac/ticket/model.py - updated version to adjust for changes in other patch: 28 insertions, 32 deletions of whitespace
comment:3 Changed 22 months ago by hoff.st@…
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 Changed 22 months ago by cboos
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 Changed 22 months ago by cboos
- Milestone set to 0.13
Changed 22 months ago by hoff.st@…
- Attachment add-ticket-class-var_to_ticket-model-py.3.patch added
patch for trac/ticket/model.py - 2nd update: 26 insertions, 25 deletions
Changed 22 months ago by hoff.st@…
- Attachment cleanup_ticket-model-py.3.patch added
patch for trac/ticket/model.py - 2nd update to adjust for changes in other patch: 28 insertions, 32 deletions of whitespace
comment:6 Changed 22 months ago by hoff.st@…
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 Changed 17 months ago by rblank
- Resolution set to fixed
- Status changed from new to closed
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 Changed 17 months ago by rblank
- Owner set to hasienda <hoff.st@…>



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