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

Update 3.3-bootstrap.php #17

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 30 additions & 20 deletions guides/3.3-bootstrap.php
@@ -1,4 +1,4 @@
<?php defined('SYSPATH') or die('No direct script access.');
<?php defined('SYSPATH') OR die('No direct script access.');

// -- Environment setup --------------------------------------------------------

Expand Down Expand Up @@ -28,47 +28,53 @@
* Set the default locale.
*
* @link http://kohanaframework.org/guide/using.configuration
* @link http://www.php.net/manual/function.setlocale
* @link http://php.net/setlocale
*/
setlocale(LC_ALL, 'en_US.utf-8');

/**
* Enable the Kohana auto-loader.
*
* @link http://kohanaframework.org/guide/using.autoloading
* @link http://www.php.net/manual/function.spl-autoload-register
* @link http://php.net/spl-autoload-register
*/
spl_autoload_register(array('Kohana', 'auto_load'));

/**
* Optionally, you can enable a compatibility auto-loader for use with
* older modules that have not been updated for PSR-0.
*
* It is recommended to not enable this unless absolutely necessary.
*/
//spl_autoload_register(array('Kohana', 'auto_load_lowercase'));

/**
* Enable the Kohana auto-loader for unserialization.
*
* @link http://www.php.net/manual/function.spl-autoload-call
* @link http://www.php.net/manual/var.configuration#unserialize-callback-func
* @link http://php.net/spl-autoload-call
* @link http://php.net/var.configuration#unserialize-callback-func
*/
ini_set('unserialize_callback_func', 'spl_autoload_call');

// -- Configuration and initialization -----------------------------------------

/**
* Set the default language
* Set the mb_substitute_character to "none".
*
* @link http://php.net/mb-substitute-character
*/
mb_substitute_character('none');

// -- Configuration and initialization -----------------------------------------

// Set the default language
I18n::lang('en-us');

/**
* Set Kohana::$environment if a 'KOHANA_ENV' environment variable has been supplied.
*
* Note: If you supply an invalid environment name, a PHP warning will be thrown
* saying "Couldn't find constant Kohana::<INVALID_ENV_NAME>"
* Replace the default HTTP protocol.
*/
if (isset($_SERVER['SERVER_PROTOCOL']))
{
HTTP::$protocol = $_SERVER['SERVER_PROTOCOL'];
}

/**
* Set Kohana::$environment if a 'KOHANA_ENV' environment variable has been supplied.
*
* Note: If you supply an invalid environment name, a PHP warning will be thrown

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Author

Choose a reason for hiding this comment

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

Kohana::$environment = KOHANA::PRODUCTION;

Copy link
Member

Choose a reason for hiding this comment

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

You're forcing ENV to be production regardless of the actual environment variables. Kohana::DEVELOPMENT is the default environment if none provided, I don't see why that would change?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bad change.

* saying "Couldn't find constant Kohana::<INVALID_ENV_NAME>"
*/
if (isset($_SERVER['KOHANA_ENV']))
{
Kohana::$environment = constant('Kohana::'.strtoupper($_SERVER['KOHANA_ENV']));
Expand All @@ -92,6 +98,10 @@
Kohana::init(array(
'base_url' => '/3.3/',
'index_file' => '',
'cache_life' => 300, // 5 minutes
'errors' => Kohana::$environment != Kohana::PRODUCTION,
Copy link
Contributor

Choose a reason for hiding this comment

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

!== no loose checks are allowed.

Copy link
Author

Choose a reason for hiding this comment

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

Give an example in which this condition is wrong. I think stringent checks should only be used when necessary. I often see programmers copying Kohana stringent checks inserted where needed and where it's not needed.
Don't forget that one of the basic ideas of PHP is the lack of strong typing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think stringent checks should only be used when necessary.

Too bad your personal opinions fly in the face of just about every best practice guide ever written by experienced programmers.

Using strict checks is required by Kohana style guide. Period. End of story.

Copy link
Author

Choose a reason for hiding this comment

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

Well then, start to rewrite existing code, and I'll add many links to places where not currently using strict checks.
https://github.com/WinterSilence/core/blob/3.4/develop/classes/Kohana/Core.php#L223
How this code got to master?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably lack of code review. Mistakes happen.

Copy link
Author

Choose a reason for hiding this comment

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

if (Kohana::$errors AND $error = error_get_last() and it? and their name is legion ...

As a result, errors:
http://github.com/kohana/core/blob/3.4/develop/classes/Kohana/Core.php#L1025
http://github.com/kohana/core/blob/3.4/develop/classes/Kohana/Core.php#L1028

That's what lead such strict instructions: programmers not thinking and use these instructions, even in cases when it is not needed. The choice of receptions should be justified and considered.

Also I don't find this practice in guide: http://kohanaframework.org/3.3/guide/kohana/conventions#constants

'profile' => Kohana::$environment != Kohana::PRODUCTION,
'caching' => Kohana::$environment == Kohana::PRODUCTION
));
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it wouldn't be bad to use Kohana::$environment comparison for setting these 3 booleans though ?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe

'profile' => Kohana::$environment === Kohana::DEVELOPMENT,
'caching' => Kohana::$environment < Kohana::DEVELOPMENT,

Just a suggestion :)

Copy link
Author

Choose a reason for hiding this comment

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

for me it's not so important. the main thing that's worked quickly and without outages due to overload. For this need use cache in PRODUCTION, other changes are minor.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't matter if it is important to you. This is a framework for many people. Do it properly.

Copy link
Author

Choose a reason for hiding this comment

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

@shadowhand Do not find fault with my words, I badly know English and you know it. Suggest your version, choose the best of the proposed. Finally, matters is the result, not who suggested it.


/**
Expand Down Expand Up @@ -124,7 +134,7 @@
'orm' => MODPATH.'orm', // Object Relationship Mapping
'unittest' => MODPATH.'unittest', // Unit testing
'userguide' => MODPATH.'userguide', // User guide and API documentation
));
));

/**
* Set the routes. Each route must have a minimum of a name, a URI and a set of
Expand Down