feat: add Boot class #8604

kenjis · 2024-03-04T00:39:35Z

~~Needs #8603~~

Description
Closes #7824

add Boot class, then index.php, spark, and Test/bootstrap.php use it
discontinue system/bootstrap.php

Checklist:

Securely signed commits
Component(s) with PHPDoc blocks, only if necessary or adds value
Unit testing, with >80% coverage
User guide updated
Conforms to style guide

github-actions · 2024-03-07T07:35:03Z

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

github-actions · 2024-03-16T00:58:13Z

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

MGatner · 2024-03-16T10:51:08Z

system/Boot.php

+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace CodeIgniter;


I always find it difficult to intercept the boot process. I don't want to overcomplicated this, but would it make sense to have an extension of this in App\ to allow devs to modify the boot process? This class could have necessary methods as final. Alternatively there could be another file like app/Config/Boot/Default.php that is always loaded alongside the environment-specific version.

If you want to modify the boot process, extend the Boot class, and run your Boot in index.php.

MGatner · 2024-03-16T10:53:22Z

system/Boot.php

+class Boot
+{
+    /**
+     * @used-by public/index.php


Did you come up with @used-by or is that an existing tag? If it doesn't already carry meaning I recommend using @friend to line up with https://github.com/DaveLiddament/php-language-extensions

The usage of @used-by is maybe not correct because public/index.php is not a Structural Element.
https://docs.phpdoc.org/guide/references/phpdoc/tags/uses.html#uses-used-by

I removed the tags, and made it just a comment.
Because both tags need a Structural Element Name, but a file is not a Structural Element.

system/Boot.php

MGatner · 2024-03-16T10:56:18Z

system/Boot.php

+            $env = $_ENV['CI_ENVIRONMENT'] ?? $_SERVER['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT');
+
+            define('ENVIRONMENT', ($env !== false) ? $env : 'production');


Seems like this could be cleaner:

Suggested change

$env = $_ENV['CI_ENVIRONMENT'] ?? $_SERVER['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT');

define('ENVIRONMENT', ($env !== false) ? $env : 'production');

$env = $_ENV['CI_ENVIRONMENT']

?? $_SERVER['CI_ENVIRONMENT']

?? getenv('CI_ENVIRONMENT')

?: 'production';

define('ENVIRONMENT', $env);

MGatner · 2024-03-16T10:57:17Z

system/Boot.php

+            $env = $_ENV['CI_ENVIRONMENT'] ?? $_SERVER['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT');
+
+            define('ENVIRONMENT', ($env !== false) ? $env : 'production');
+            unset($env);


This scopes out immediately anyways.

Suggested change

unset($env);

MGatner · 2024-03-16T11:02:02Z

system/Boot.php

+                throw FrameworkException::forMissingExtension(implode(', ', $missingExtensions));
+            }
+
+            unset($missingExtensions);


Scopes out immediately.

Suggested change

unset($missingExtensions);

MGatner · 2024-03-16T11:04:09Z

system/Boot.php

+        static::loadCommonFunctions();
+        static::loadAutoloader();
+        static::setExceptionHandler();
+        static::checkMissingExtensions();


This feels awfully low down the stack. We really don't need these extensions in any of the prior methods? I recognize that the FrameworkException relies on Autoloader and probably Exception Handling, but we could convert it to a simpler echo + exit similar to the environment error.

It means we stop providing the translated error message.

Changed the method order, and changed it to echo + exit.

system/bootstrap.php

We should write FQSEN, but a PHP file is not a FQSEN. @uses [FQSEN] [<description>] https://docs.phpdoc.org/guide/references/phpdoc/tags/uses.html#uses-used-by

The translated error message is no longer provided. This is because the checking must be done at a very early stage.

MGatner

Looks good

kenjis · 2024-03-26T21:18:13Z

@MGatner Thank you for the review!

lonnieezell · 2024-03-26T21:25:05Z

I'm curious - is this part of a set of changes coming or was it just to clean things up?

Only ask because if it was just to clean it up then we just added a slight bit more overhead and performance cost by using a class where a plain script was doing everything just fine. Granted, it's pretty much negligible but it is a trend we're doing and we're also more and more concerned about performance.

kenjis · 2024-03-26T21:57:27Z

This PR is a part of a series that ends with #8610.

I changed the scripts to a class because the scripts were too long and difficult to understand.
There are practically three scripts, for web, spark, and testing, and they are slightly different.

If the performance loss is really important, we could convert the class to a plain script.

kenjis added enhancement PRs that improve existing functionalities 4.5 labels Mar 4, 2024

kenjis marked this pull request as draft March 4, 2024 00:39

This was referenced Mar 4, 2024

feat: add Config\Optimize #8605

Merged

feat: add Boot class and refactor index.php/spark/CodeIgniter::initialize() #8558

Closed

github-actions bot added the stale Pull requests with conflicts label Mar 7, 2024

kenjis force-pushed the feat-add-Boot-class branch from 0c2f6d9 to 4b4c026 Compare March 7, 2024 07:41

kenjis removed the stale Pull requests with conflicts label Mar 7, 2024

kenjis force-pushed the feat-add-Boot-class branch from 4b4c026 to 46c91ca Compare March 10, 2024 10:00

kenjis mentioned this pull request Mar 11, 2024

fix: move Kint loading to Autoloader #8603

Merged

5 tasks

github-actions bot added the stale Pull requests with conflicts label Mar 16, 2024

kenjis force-pushed the feat-add-Boot-class branch from 46c91ca to 5ff0397 Compare March 16, 2024 00:58

kenjis removed the stale Pull requests with conflicts label Mar 16, 2024

kenjis marked this pull request as ready for review March 16, 2024 01:09

MGatner requested changes Mar 16, 2024

View reviewed changes

kenjis added 13 commits March 17, 2024 08:41

refactor: extract Boot class

1671882

refactor: move code to Boot class

5338b3d

refactor: use Boot class in Test/bootstrap.php

529eb95

refactor: fix method name

5691689

refactor: remove rtrim()

c5191a0

docs: update comments

41db766

docs: add user guide

7b81bb0

docs: add @codeCoverageIgnore

c335731

docs: remove space

4a430ea

docs: remove @Used-By

4cd1a70

We should write FQSEN, but a PHP file is not a FQSEN. @uses [FQSEN] [<description>] https://docs.phpdoc.org/guide/references/phpdoc/tags/uses.html#uses-used-by

refactor: extract boot() method

a95c6f1

refactor: improve readability

1022a33

refactor: remove unneeded unset()

61bf440

kenjis force-pushed the feat-add-Boot-class branch from 9b8fc1b to 61bf440 Compare March 16, 2024 23:52

kenjis added 3 commits March 17, 2024 10:06

refactor: move checkMissingExtensions() up

dc9caae

The translated error message is no longer provided. This is because the checking must be done at a very early stage.

refactor: early return

a5ee340

fix: change HTTP status code from 200 to 503

44583a4

kenjis force-pushed the feat-add-Boot-class branch from d64746a to 08dd2c6 Compare March 17, 2024 01:11

kenjis requested a review from MGatner March 17, 2024 01:12

kenjis force-pushed the feat-add-Boot-class branch from 08dd2c6 to e40baa2 Compare March 17, 2024 03:45

feat: "system/bootstrap.php" shows upgrade error message

842fd6e

kenjis force-pushed the feat-add-Boot-class branch from e40baa2 to 842fd6e Compare March 17, 2024 03:48

kenjis added the breaking change Pull requests that may break existing functionalities label Mar 17, 2024

MGatner approved these changes Mar 26, 2024

View reviewed changes

kenjis merged commit 39210be into codeigniter4:4.5 Mar 26, 2024
47 checks passed

kenjis deleted the feat-add-Boot-class branch March 26, 2024 21:17

kenjis mentioned this pull request Mar 31, 2024

Dev: move code from index.php to bootstrap.php? #7824

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add Boot class #8604

feat: add Boot class #8604

kenjis commented Mar 4, 2024 •

edited

Loading

github-actions bot commented Mar 7, 2024

github-actions bot commented Mar 16, 2024

MGatner Mar 16, 2024

kenjis Mar 16, 2024

MGatner Mar 16, 2024

kenjis Mar 16, 2024

kenjis Mar 16, 2024

MGatner Mar 16, 2024

kenjis Mar 16, 2024

MGatner Mar 16, 2024

kenjis Mar 16, 2024

MGatner Mar 16, 2024

kenjis Mar 16, 2024

MGatner Mar 16, 2024

kenjis Mar 17, 2024

kenjis Mar 17, 2024

MGatner left a comment

kenjis commented Mar 26, 2024

lonnieezell commented Mar 26, 2024

kenjis commented Mar 26, 2024

		$env = $_ENV['CI_ENVIRONMENT'] ?? $_SERVER['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT');

		define('ENVIRONMENT', ($env !== false) ? $env : 'production');

feat: add Boot class #8604

feat: add Boot class #8604

Conversation

kenjis commented Mar 4, 2024 • edited Loading

github-actions bot commented Mar 7, 2024

github-actions bot commented Mar 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MGatner left a comment

Choose a reason for hiding this comment

kenjis commented Mar 26, 2024

lonnieezell commented Mar 26, 2024

kenjis commented Mar 26, 2024

kenjis commented Mar 4, 2024 •

edited

Loading