#12787 closed task (fixed)
Remove deprecated code (Trac 1.5.1) — at Version 26
Reported by: | Ryan J Ollos | Owned by: | Ryan J Ollos |
---|---|---|---|
Priority: | normal | Milestone: | 1.5.1 |
Component: | general | Version: | |
Severity: | normal | Keywords: | |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: |
Removed deprecated code:
|
||
Internal Changes: |
Change History (26)
follow-up: 3 comment:1 by , 7 years ago
comment:2 by , 7 years ago
Changes in log:rjollos.git:t12787_remove_deprecated_code. It will be quite a while before the changes are committed, but they could use some review. All tests pass for me, so if there are issues then it will be an opportunity to improve test coverage.
There's one TODO in chrome.py
: Chrome.render_template.
I'll rebase and post revised changes periodically.
comment:3 by , 7 years ago
Replying to Ryan J Ollos:
Is the intention that plugins in 1.5.1 and later should always implement the method signature
post_process_request(self, req, template, data, metadata=None)
, or isdef post_process_request(req, template, data)
also allowed as a method signature for a class implementingIRequestFilter
?
I think I overlooked that metadata=None
was for backward-compatibility, and in the 1.5.1+ API it should be a positional, not a keyword, argument. Therefore the correct signature is post_process_request(self, req, template, data, metadata)
.
Additional changes in [ff9c6753/rjollos.git]. For the case in which the process_request
does not return metadata
, an empty dict is passed to the post_process_request
methods.
comment:4 by , 7 years ago
Rebased and additional changes in log:rjollos.git:t12787_remove_deprecated_code.1.
comment:5 by , 7 years ago
Scheduled EnvironmentStub.clear_component_registry
and EnvironmentStub.restore_component_registry
for removal in 1.5.1: r15980.
comment:6 by , 7 years ago
Milestone: | next-major-releases → 1.5.1 |
---|
follow-up: 9 comment:8 by , 7 years ago
Rebased changes in log:rjollos.git:t12787_remove_deprecated_code.3. Tests pass with and without Genshi.
Is the following needed when support for Genshi is removed?: trunk/trac/util/html.py@16037:712-713#L698.
Should we allow the metadata
argument of process_request
to be None
with removal of Genshi support? From TracDev/PortingFromGenshiToJinja#IRequestHandler:
Note that as long as we have to support the legacy Genshi templates, a
None
value passed as third argument won't be interpreted as an emptydict
, but rather as an emptycontent_type
.
follow-up: 10 comment:9 by , 7 years ago
Replying to Ryan J Ollos:
Rebased changes in log:rjollos.git:t12787_remove_deprecated_code.3. Tests pass with and without Genshi.
Is the following needed when support for Genshi is removed?: trunk/trac/util/html.py@16037:712-713#L698.
No, I don't think so.
Should we allow the
metadata
argument ofprocess_request
to beNone
with removal of Genshi support? From TracDev/PortingFromGenshiToJinja#IRequestHandler:Note that as long as we have to support the legacy Genshi templates, a
None
value passed as third argument won't be interpreted as an emptydict
, but rather as an emptycontent_type
.
The idea was that once we no longer need to play subtle games to differentiate between the two engines, return 'template.html', data
should become equivalent to return 'template.html', data, {}
.
comment:10 by , 7 years ago
Replying to Christian Boos:
Is the following needed when support for Genshi is removed?: trunk/trac/util/html.py@16037:712-713#L698.
No, I don't think so.
I've removed the line and will post the changes after some more time passes and another rebase is needed.
Should we allow the
metadata
argument ofprocess_request
to beNone
with removal of Genshi support? From TracDev/PortingFromGenshiToJinja#IRequestHandler:
I misspoke here. I was looking at render_template and wondering if we should allow metadata
to be None
. We'd just need to add:
metadata = metadata or {}
comment:13 by , 6 years ago
Rebased with changes from #13042: log:rjollos.git:t12787_remove_deprecated_code.6.
comment:16 by , 5 years ago
That clean-up looks good to me, and simplifies things a lot. However, as trunk seems to be still 1.3dev, I suppose you plan to branch 1.4-stable first, right?
comment:17 by , 5 years ago
Yeah, I won't put on trunk until trunk is 1.5.1dev. In the meantime I'll continue to rebase the branch to avoid a bigger painful rebase later.
follow-up: 19 comment:18 by , 5 years ago
Rebased against r16880 in log:rjollos.git:t12787_remove_deprecated_code.9.
follow-up: 20 comment:19 by , 5 years ago
Replying to Ryan J Ollos:
Rebased against r16880 in log:rjollos.git:t12787_remove_deprecated_code.9.
Isn't the add_script(req, 'common/js/excanvas.js')
only needed for IE?
comment:20 by , 5 years ago
Replying to Christian Boos:
Isn't the
add_script(req, 'common/js/excanvas.js')
only needed for IE?
Yeah, that looks to be the case. I misunderstood the ie_if
logic on first read.
comment:21 by , 5 years ago
comment:19 issue fixed and rebased against r16880 in log:rjollos.git:t12787_remove_deprecated_code.10.
comment:24 by , 5 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:25 by , 5 years ago
Type: | defect → task |
---|
comment:26 by , 5 years ago
API Changes: | modified (diff) |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Changes committed in [17091-17100].
Is the intention that plugins in 1.5.1 and later should always implement the method signature
post_process_request(self, req, template, data, metadata=None)
, or isdef post_process_request(req, template, data)
also allowed as a method signature for a class implementingIRequestFilter
?