Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ez_http_tag_location → ez_http_cache_tag_location #1297

Closed
wants to merge 2 commits into from

Conversation

adriendupuis
Copy link
Contributor

@adriendupuis adriendupuis commented Jan 29, 2021

Question Answer
JIRA Ticket N/A
Versions Ibexa DXP (eZ Platform v3), ezsystems/ezplatform-http-cache v2.1.3+

Replace ez_http_tag_location (kept for 2.5 BC) with ez_http_cache_tag_location.

Copy link
Member

@adamwojs adamwojs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ezsystems/documentation-team Could you please add ez_http_cache_tag_location and other HTTP cache related function to https://doc.ibexa.co/en/latest/guide/twig_functions_reference/ ?

@dabrt dabrt self-assigned this Feb 1, 2021

For full content tagging when inline rendering, use the following:

``` html+twig
{{ ez_http_tag_location(location) }}
{{ ez_http_cache_tag_location(location) }}
```

2\. `ez_http_tag_relation_ids()` or `ez_http_tag_relation_location_ids()`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely, these functions should also have the ez_http_cache prefix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamwojs
Could you please confirm that
ez_http_tag_relation_ids() and ez_http_tag_relation_location_ids()
should be updated in the docs to
ez_http_cache_tag_relation_ids() and ez_http_cache_tag_relation_location_ids()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vidarl ez_http_cache_tag_location doesn't exist (yet).And the goal of ezsystems/ezplatform-http-cache#143 wasn't to create it. Could it be done in another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vidarl

ez_http_cache_tag_relation_ids and ez_http_cache_tag_relation_location_ids added in ezsystems/ezplatform-http-cache#143

Doc updated in 4f9b410 to have this two new Twig functions with the ez_http_cache_tag_ prefix.

Copy link
Contributor

@vidarl vidarl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we still list the function names that is deprecated? so that people searching for it still finds it?

Not sure how the policy is for deprecated things, honestly @dabrt

@dabrt
Copy link
Contributor

dabrt commented Feb 17, 2021

Not sure how the policy is for deprecated things, honestly @dabrt

We do not mention them in the docs, the policy is to list deprecations, like here:
https://doc.ibexa.co/en/master/releases/ez_platform_v3.0_deprecations/#functions-renamed

For whatever reason these deprecations have not been reported at v3.0 and this branch has already been abandoned. At this point we should simply replace the deprecated signatures with proper ones.

@dabrt
Copy link
Contributor

dabrt commented Feb 17, 2021

Hi @adriendupuis ,
Is there any traction in this PR? Are we waiting for further approvals?

@adriendupuis
Copy link
Contributor Author

Hi @adriendupuis ,
Is there any traction in this PR? Are we waiting for further approvals?

Hello @dabrt
There was some discussions on the code part about deprecating more than this function. I have some rebasing to do regarding since which version it should have been deprecated. I didn't find the time to finish this yet.

@dabrt
Copy link
Contributor

dabrt commented Jul 27, 2021

Bonjour @adriendupuis
Please either merge this PR or close it if no longer needed.

@adriendupuis
Copy link
Contributor Author

Those Twig function names have been updated in #1403

@adriendupuis adriendupuis deleted the adriendupuis-patch-1 branch July 28, 2021 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants