Skip to content

Conversation

@kenjis
Copy link
Member

@kenjis kenjis commented Mar 4, 2024

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

@kenjis kenjis added enhancement PRs that improve existing functionalities 4.5 labels Mar 4, 2024
@kenjis kenjis marked this pull request as draft March 4, 2024 00:39
@github-actions github-actions bot added the stale Pull requests with conflicts label Mar 7, 2024
@github-actions
Copy link

github-actions bot commented Mar 7, 2024

👋 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

@kenjis kenjis force-pushed the feat-add-Boot-class branch from 0c2f6d9 to 4b4c026 Compare March 7, 2024 07:41
@kenjis kenjis removed the stale Pull requests with conflicts label Mar 7, 2024
@kenjis kenjis force-pushed the feat-add-Boot-class branch from 4b4c026 to 46c91ca Compare March 10, 2024 10:00
@kenjis kenjis mentioned this pull request Mar 11, 2024
5 tasks
@github-actions
Copy link

👋 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 github-actions bot added the stale Pull requests with conflicts label Mar 16, 2024
@kenjis kenjis force-pushed the feat-add-Boot-class branch from 46c91ca to 5ff0397 Compare March 16, 2024 00:58
@kenjis kenjis removed the stale Pull requests with conflicts label Mar 16, 2024
@kenjis kenjis marked this pull request as ready for review March 16, 2024 01:09
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter;
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

system/Boot.php Outdated
$env = $_ENV['CI_ENVIRONMENT'] ?? $_SERVER['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT');

define('ENVIRONMENT', ($env !== false) ? $env : 'production');
unset($env);
Copy link
Member

Choose a reason for hiding this comment

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

This scopes out immediately anyways.

Suggested change
unset($env);

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@kenjis 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
The translated error message is no longer provided.
This is because the checking must be done at a very early stage.
@kenjis kenjis force-pushed the feat-add-Boot-class branch from d64746a to 08dd2c6 Compare March 17, 2024 01:11
@kenjis kenjis requested a review from MGatner March 17, 2024 01:12
@kenjis kenjis force-pushed the feat-add-Boot-class branch from 08dd2c6 to e40baa2 Compare March 17, 2024 03:45
@kenjis kenjis force-pushed the feat-add-Boot-class branch from e40baa2 to 842fd6e Compare March 17, 2024 03:48
@kenjis kenjis added the breaking change Pull requests that may break existing functionalities label Mar 17, 2024
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Looks good

@kenjis kenjis merged commit 39210be into codeigniter4:4.5 Mar 26, 2024
@kenjis kenjis deleted the feat-add-Boot-class branch March 26, 2024 21:17
@kenjis
Copy link
Member Author

kenjis commented Mar 26, 2024

@MGatner Thank you for the review!

@lonnieezell
Copy link
Member

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
Copy link
Member Author

kenjis commented Mar 26, 2024

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.

@totoprayogo1916
Copy link
Contributor

I have moved the location of the .env file in CI v4.4.8.
How can I set the .env file location in the latest version?

@totoprayogo1916
Copy link
Contributor

or we can add path for .env?

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

Labels

breaking change Pull requests that may break existing functionalities enhancement PRs that improve existing functionalities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants