Opened 6 years ago

Closed 5 years ago

Document which components are required

Reported by: Owned by: Ryan J Ollos Steffen Hoffmann normal TagsPlugin normal 1.0

I was looking at the plugin admin page the other day and thinking it would be useful if we could indicate in the docstrings which components are required.

In the bigger scheme of things I'm considering trac:#11567, and trying to figure out what might work well for plugins, and this seemed like a nice refined and widely used plugin to apply the changes to.

comment:1 Changed 6 years ago by Ryan J Ollos

Description: modified (diff) changed from anonymous to Ryan J Ollos

comment:2 follow-up:  4 Changed 6 years ago by Steffen Hoffmann

I see your point, and I've been looking after information regarding essential and optional plugin components myself many times before.

Easiest way would be clearly visible text marker in doc-strings. A generic schema would be required, and discussion about it could go on here, sure.

Rationale:

• plugin components are always optional looking from Trac core
• still one would like to highlight essential and optional, maybe even extra components
• legend proposal:
[main]
basics and core plugin features like API, db backend
[opt]
recommended, but non-essentials like wiki macros, timeline provider
[extra]
"chrome" stuff, configuration admin panel, other rather exotic components
• and probably explain a bit more about the "why" in trailing text

I'd even like the idea of required components, but for plugins I see the benefit of being able to disable plugins completely. As far as I can see, this would be impossible for components marked with required = True, right?

comment:3 Changed 6 years ago by Steffen Hoffmann

In 13859:

Component dependency checks are better performed at runtime, and Genshi is
required for Trac anyway.

Following Ryan's recommendation in http://trac-hacks.org/wiki/DevGuide.

comment:4 in reply to:  2 Changed 6 years ago by Ryan J Ollos

I see your point, and I've been looking after information regarding essential and optional plugin components myself many times before.

Your proposal for docstring markers looks good to me. Adding it to TagsPlugin for the 0.8 release might be a good opportunity to get feedback before adding the markers to docstrings in other plugins.

I'd even like the idea of required components, but for plugins I see the benefit of being able to disable plugins completely. As far as I can see, this would be impossible for components marked with required = True, right?

I have always wondered what the effect of required = True would be for a plugin. It appears that its only effect is to disable the checkbox for that component on the admin panel: browser:/tags/trac-1.0.1/trac/admin/templates/admin_plugins.html@:168,173#L167 trac.about.AboutModule is a required component, but adding trac.about.AboutModule = disabled in trac.ini still results in the Component being disabled. I see the same behaviour when adding required = True to a plugin - the admin panel checkbox is disabled, but the component disabled/enabled state is unaffected. So as you've suggested, I can't see that the attribute would be useful for plugins.

I am hoping that we can able to add Component dependencies to the Trac core eventually. For example, a required_if attribute on a Component class would cause a Component to be conditionally enabled if another Component is enabled. Maybe that will eventually be handled in trac:#11567.

Changed 6 years ago by Steffen Hoffmann

component tags in TagsPlugin according to proposal

comment:5 Changed 6 years ago by Steffen Hoffmann

This is how it looks like in Trac 1.1.1dev:

comment:6 Changed 6 years ago by Steffen Hoffmann

With the look of a non-maintainer this is a documentation improvement indeed.

Hopefully it will help to prevent issues like #11697, so I'll hurry to add a similar clarification to AccountManagerPlugin as well.

Last edited 6 years ago by Steffen Hoffmann (previous) (diff)

comment:7 follow-up:  9 Changed 6 years ago by Ryan J Ollos

Looks nice. Two notes:

• A replacement enhance -> enhanced is need in [13865#file2].
• Not having looked at the code, I'm not sure what TagPolicy provides. Is there another sentence you could add to explain the permission checks it performs?

comment:8 Changed 6 years ago by Steffen Hoffmann

In 13867:

TagsPlugin: Refit of configuration options inherited from KeywordSuggestPlugin, refs #1344, #3816, #4201 and #11690.

Included are minor changes for component doc-strings as follow-up to [13865].

comment:9 in reply to:  7 Changed 6 years ago by Steffen Hoffmann

Looks nice. Two notes:

Seen that, and rephrased in [13867].

• Not having looked at the code, I'm not sure what TagPolicy provides. Is there another sentence you could add to explain the permission checks it performs?

Not just a sentence, it will require to read the mailing-list discussion associated with the idea for this policy put an abstract of it into a dedicated wiki page, possibly even a 'README' file included in source distributions.

Anyway, the idea is simple: allow fine-grained permission alterations by tags composed from <user_or_group_name>:<action> pairs like follows:

anonymous:-view in a wiki page is meant to remove WIKI_VIEW permission from anonymous sessions only for this specific page, while wiki pages remain visible to guests in general, joe:edit - in a wiki page is meant to give user joe WIKI_ADMIN permissions for this specific page, while WIKI_EDIT is granted to that user as all other authenticated users in general

comment:10 Changed 6 years ago by Steffen Hoffmann

In 13882:

DiscussionPlugin: Major unification of timeline discussion event provider code, refs #6783.

Replacing string tuples with true Trac Resource objects is required i. e.
to let this event provider work with tractags.web_ui.TagTimelineEventFilter.

Other changes include

• more imports cleanup and PEP8 changes (line-wrap, indentation)
• making DEBUG log entries more explicit
• simplification of string-replacements and refit of component doc-string according to component relevance markup proposal (see #11690 for TagsPlugin)
• more readable 'message_<messageID>' anchor name format for direct links to individual messages (was: 'message<messageID>')

comment:11 Changed 5 years ago by Ryan J Ollos

Resolution: → fixed new → closed

In 14945:

0.8: Tag 0.8 release

Refs #12415.

comment:12 Changed 4 years ago by Ryan J Ollos

Test failures that follow r13865 are discussed in #12056.

comment:13 Changed 4 years ago by Ryan J Ollos

Documented some of the discussion from this ticket in DevGuide@24.

Modify Ticket

Change Properties