Modify

Opened 11 years ago

Closed 11 years ago

#10883 closed task (fixed)

Minor suggestions from testing and code review

Reported by: Ryan J Ollos Owned by: Zack
Priority: normal Component: TracComponentAliasPlugin
Severity: normal Keywords:
Cc: Trac Release:

Description

I've tested and code-reviewed the plugin, and will provide some suggested changes in a forthcoming patch. I can push the patch to the repository directly if you give approval.

Attachments (3)

ManagePluginsPanel.png (14.3 KB) - added by anonymous 11 years ago.
ErrorOnDelete.png (21.2 KB) - added by anonymous 11 years ago.
t10883-r12636-1.patch (2.1 KB) - added by anonymous 11 years ago.
Patch against r12636 of the trunk.

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by anonymous

Attachment: ManagePluginsPanel.png added

Changed 11 years ago by anonymous

Attachment: ErrorOnDelete.png added

comment:1 Changed 11 years ago by Ryan J Ollos

Here are some changes contained the patch:

  • Removed some unused code: the plugin doesn't have any logic in the ITicketManipulator implementation.
  • Removed unused import of find_packages in setup.py.
  • Fixed error when trying to delete ticket because there was no implementation of ITicketChangeListener.ticket_deleted (see screen capture below).
  • Updated some strings in setup.py to be more descriptive, since they are shown on the plugin admin panel (see screen capture below).



I also edited the project wiki page slightly to try to clarify a few things.

  • Cut and paste of the example didn't work because # isn't treated as a comment in the ini file, rather ; is.
  • Added ticket-custom configuration to the example.
  • Clarified that the plugin is implemented server-side, so the component won't be changed until the ticket is submitted.
  • Clarified that the alias is only valid for select and radio custom field types. Those are the only two custom field types that have an options field: TracTicketsCustomFields#AvailableFieldTypesandOptions.
  • Removed 0.12 tag from the wiki page. This was probably an artifact of when the plugin project was created.

See changes here.

I found the use of the term alias a bit confusing. Do you have a specific use-case for the plugin that might help clarify that?

Changed 11 years ago by anonymous

Attachment: t10883-r12636-1.patch added

Patch against r12636 of the trunk.

comment:2 in reply to:  1 Changed 11 years ago by Zack

Replying to rjollos:

I found the use of the term alias a bit confusing. Do you have a specific use-case for the plugin that might help clarify that?

Well, we create our components to match the svn project name, but the end user creating a ticket may not know which component they need to select. Now we offer a list of "applications" that they can choose from and that maps to a component. That is where this plugin does the mapping.

comment:3 Changed 11 years ago by Zack

Patch applied. Thanks!

comment:4 Changed 11 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

I'll go ahead and close this then, and just note that [12676] was the changeset in which the patch was applied.

I saw your changes in [13051], and would agree that using ITicketManipulator rather than ITicketChangeListener seems like the right thing to do here. Nice work.

Btw, trac-hacks supports t:CommitTicketUpdater, so you can automatically reference tickets in your commit messages if you'd like.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Zack.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.