Code review

Description

As recent development has been tracked in #6783, but going totally out-of-scope now, this ticket is meant to collect references and additional comment regarding this ongoing effort for tracdiscussion-0.9.

Attachments (0)

Oldest firstNewest firstThreaded

Show commentsShow property changes

Change History (38)

More PEP8 changes (doc-string quotation placement, line-wrap), comment review,
and complete preparation of URLs to messages as anchors in the corresponding
topic (parent resource) page by get_resource_url() for the discussion
Trac Resource domain as planned in [13882].

This is the initial split as done to other plugins before for improved
maintainability of db schema-specific code. Cutting method count per class
as well as overall lines per class down to more manageable size will help
anyway.

forum_cols was added as an attribute of the DiscussionDb class, but _get_forum which references forum_cols via self.forum_cols was moved to DiscussionApi. It looks like that might result in an AttributeError.

forum_cols was added as an attribute of the DiscussionDb class, but the _get_forum method which references the forum_cols attribute via the call self.forum_cols was moved to DiscussionApi. It looks like that might result in an AttributeError.

I've been uneasy about this too, however the class inheritance of DiscussionDb blends all attributes nicely to DiscussionApi, so I just did the same for other column name tuples as well.

Apart from usual PEP8 issues some SQL queries could be replaced with existing
method calls for robustness and saving lines in the module.
Remove debug logging, where resource changes in context are obvious anyway.

This issue in model._get_items() went through on first review for [13919].
Luckily seeing the same thing done right in get_topics pointed it out.
In a similar notion unregistered users are retrieved by set difference instead
of iteration over topic['subscribers'] in _prepare_topic() too.

Add Comment

This ticket has been modified since you started editing. You should review the
other modifications which have been appended above,
and any conflicts shown in the preview below.
You can nevertheless proceed and submit your changes if you wish so.