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

Remove deprecated HydePage::$canonicalUrl property #1256

Merged
merged 15 commits into from
Mar 12, 2023

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Mar 12, 2023

The idea is that the internal data state should be as portable as possible. Having a canonicalUrl as part of the internal data structure breaks this portability as it is in many cases entirely dependent on the environment the data is used in. This means that it should not be part of the internal data structure, nor its serialized form. It's much better suited as a runtime accessor method. Thus, this PR removes the deprecated property, and replaces it with a new public method that uses the same logic ported from the page factory creator so the end result is still the same.

@codecov
Copy link

codecov bot commented Mar 12, 2023

Codecov Report

Merging #1256 (2fbffa8) into master (aadbf6c) will not change coverage.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##              master     #1256     +/-   ##
=============================================
  Coverage     100.00%   100.00%             
- Complexity      1317      2632   +1315     
=============================================
  Files            152       304    +152     
  Lines           3459      6908   +3449     
=============================================
+ Hits            3459      6908   +3449     
Impacted Files Coverage Δ
...rk/src/Framework/Factories/HydePageDataFactory.php 100.00% <ø> (ø)
...rc/Framework/Features/Metadata/PageMetadataBag.php 100.00% <100.00%> (ø)
...mework/Features/XmlGenerators/RssFeedGenerator.php 100.00% <100.00%> (ø)
packages/framework/src/Pages/Concerns/HydePage.php 100.00% <100.00%> (ø)

... and 152 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@caendesilva caendesilva force-pushed the remove-deprecated-canonicalUrl-field branch from 257bb44 to 013bf0e Compare March 12, 2023 18:51
@caendesilva caendesilva marked this pull request as ready for review March 12, 2023 19:36
@caendesilva caendesilva merged commit 2ee4062 into master Mar 12, 2023
@caendesilva caendesilva deleted the remove-deprecated-canonicalUrl-field branch March 12, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant