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

FIX Maintenance module extension now provides CWP proxy information for HTTP requests #125

Conversation

robbieaverill
Copy link
Contributor

@NightJar NightJar merged commit 3ad235e into silverstripe:2.1 Jun 26, 2018
@NightJar NightJar deleted the pulls/2.1/maintenance-proxy-config branch June 26, 2018 03:52
use SilverStripe\Core\Environment;
use SilverStripe\Core\Extension;

if (!class_exists(SiteSummary::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI this does nothing as the class below is still compiled...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? This is a common pattern in SilverStripe

Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure... I just tested it with this gist: https://gist.github.com/ScopeyNZ/7ad38ff41f3bdc03dd2141474e0793df

Copy link
Contributor

@NightJar NightJar Jun 26, 2018

Choose a reason for hiding this comment

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

I find this information mildly perplexing. As Robbie said, it is (or at least was) all throughout the core code, and various modules. Is it perhaps a PHP version thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed this yesterday - I'm pretty sure it's needed for when the ClassManifest gets regenerated, but only when extending a class that doesn't always exist. In this case it's not required since Extension is always there

Copy link
Contributor

@ScopeyNZ ScopeyNZ Jun 27, 2018

Choose a reason for hiding this comment

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

Well I'm learning a lot about the PHP compiler today. Logical expressions (non-declarative code) are usually ignored at compile time. The compiler considers all class definitions.

  1. If the extended class exists then the class is defined and the if statement does nothing.
  2. If the extended class does not exist but you have the if statement then the if statement is respected and you don't get an error unless you try to instantiate the class (that's doing the extending). The error will be that the class that's doing the extending does not exist.
  3. If you don't have the if statement then PHP will fatal at compile time and let you know the extended class does not exist.

PHP...

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.

3 participants