Ticket #10883 (closed task: fixed)

Opened 3 months ago

Last modified 3 weeks ago

Minor suggestions from testing and code review

Reported by: rjollos Assigned to: zshahan
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

ManagePluginsPanel.png (14.3 kB) - added by anonymous on 02/20/13 01:28:48.
ErrorOnDelete.png (21.2 kB) - added by anonymous on 02/20/13 02:16:14.
t10883-r12636-1.patch (2.1 kB) - added by anonymous on 02/20/13 02:28:13.
Patch against r12636 of the trunk.

Change History

02/20/13 01:28:48 changed by anonymous

  • attachment ManagePluginsPanel.png added.

02/20/13 02:16:14 changed by anonymous

  • attachment ErrorOnDelete.png added.

(follow-up: ↓ 2 ) 02/20/13 02:26:44 changed by rjollos

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?

02/20/13 02:28:13 changed by anonymous

  • attachment t10883-r12636-1.patch added.

Patch against r12636 of the trunk.

(in reply to: ↑ 1 ) 03/01/13 20:00:14 changed by zshahan

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.

03/01/13 20:20:32 changed by zshahan

Patch applied. Thanks!

05/04/13 23:04:23 changed by rjollos

  • status changed from new to closed.
  • resolution set to fixed.

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.

Add/Change #10883 (Minor suggestions from testing and code review)




Change Properties
Action