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

Deprecating X-Content-Encoded-By #1233

Merged
merged 3 commits into from Jun 28, 2013

Conversation

Projects
None yet
2 participants
@piotr-cz
Copy link
Contributor

piotr-cz commented Jun 4, 2013

as per discussion jgen: X-Content-Encoded-By

Before patch when setting Gzip Page Compression: Yes
X-Content-Encoded-By:Joomla! 1.6

After patch when setting Gzip Page Compression: Yes + Show Joomla! Version: Yes
X-Content-Encoded-By:Joomla! 3.0.3 (JVERSION)

Issue tracker: #31080

Discussion: Joomla! General Development: X-Content-Encoded-By

Piotr
@piotr-cz

This comment has been minimized.

Copy link
Contributor

piotr-cz commented Jun 20, 2013

ATM patch doesn't pass two JApplicationWebTest:

What about removing the lines 1 => array('name' => 'X-Content-Encoded-By', 'value' => 'Joomla'), since the use of X-Content-Encoded-By is marked as deprecated?

$this->setHeader('X-Content-Encoded-By', 'Joomla');
/*
* Header to be removed in 14.1 (Platform) & 3.5 (CMS)

This comment has been minimized.

@mbabker

mbabker Jun 22, 2013

Member

Since the Platform has ceased, you don't need to include a reference to it here. Also, could you change the CMS version to 4.0? To me, it reads that we'll remove it in 3.5.

This comment has been minimized.

@piotr-cz

piotr-cz Jun 22, 2013

Contributor

That's for a reason. Why not 3.5?

  • the issue is being reported since 1.5 days #22495,
  • there are no known backward compatibility issues

Is it that deprecating is scheduled for 4.0?

Should the comment look like that then?

/*
 * @deprecated  4.0  Use JResponse::setHeader() instead
 */

This comment has been minimized.

@mbabker

mbabker Jun 22, 2013

Member

It will be a backwards compatibility break, as minuscule as it is, and our
policy is to only do those (as practical) at major releases. If it weren't
for that, I'm pretty sure nobody would miss the header if we deleted it now.

Just make the comment "Header will be removed at 4.0" and it'll be good.
@deprecated tags are usually only used in class/method/var doc blocks.

On Saturday, June 22, 2013, Piotr wrote:

In libraries/joomla/application/web.php:

@@ -390,7 +390,14 @@ protected function compress()

            // Set the encoding headers.
            $this->setHeader('Content-Encoding', $encoding);
  •           $this->setHeader('X-Content-Encoded-By', 'Joomla');
    
  •           /*
    
  •            \* Header to be removed in 14.1 (Platform) & 3.5 (CMS)
    

That's for a reason. Why not 3.5?

Is it that deprecating is scheduled for 4.0?

Should the comment look like that then?

/* * @deprecated 4.0 Use JResponse::setHeader() instead */


Reply to this email directly or view it on GitHubhttps://github.com//pull/1233/files#r4831566
.

Piotr
@piotr-cz

This comment has been minimized.

Copy link
Contributor

piotr-cz commented Jun 26, 2013

Comments changes as per @mbabker's advice: // Header will be removed at 4.0

mbabker added a commit that referenced this pull request Jun 28, 2013

Merge pull request #1233 from piotr-cz/patch_XContentEncodedBy
Deprecating X-Content-Encoded-By

@mbabker mbabker merged commit 1633a9b into joomla:master Jun 28, 2013

1 check passed

default The Travis CI build passed
Details

@piotr-cz piotr-cz deleted the piotr-cz:patch_XContentEncodedBy branch Jun 28, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment