Feature bump ci to 4.6.0 #4197
Feature bump ci to 4.6.0 #4197
Conversation
Signed-off-by: objecttothis <objecttothis@gmail.com>
- Bump codeigniter4/framework to 4.6.0 - Bump codeIgniter/coding-standard to ^1.8 - Bump codeigniter4/devkit to ^1.3 - Updated framework files required by CI4.6.0 - Removed Deprecated variables - Added new file in the repo from framework Signed-off-by: objecttothis <objecttothis@gmail.com>
BudsieBuds
force-pushed
the
feature-bump-CI-to-4.6.0
branch
from
77423e0 to
5b430aa
Compare
Updates for PHP 8.4 support introduced with the upgrade to CodeIgniter 4.6.x
|
I've added a commit to indicate support for PHP 8.4 after updating to CI 4.6.x. However the linter is failing, maybe just updating the CI version is not enough for 8.4 support? |
If you look at the log from the phplint run on PHP 8.4 they are all deprecation warnings, so the linter might be a little aggressive. I don't know if we can configure it to pass on warnings or not. That said, in its current configuration it won't pass 8.4 tests unless we also change the code referenced in those deprecation warnings. I don't want to do it in this PR though. |
|
@BudsieBuds and @jekkos I think we should turn off the PHP 8.4 linter for this one because it's failing on deprecation warnings. |
|
Maybe it's best to enable the 8.4 linter and fix issues in one go. If that out of scope for this PR then let's disable it again |
|
I agree that we should make it 8.4 compatible but in a separate PR. |
|
You can delete my commit and merge. It'll remove the linter, and the documentation will still say PHP 8.1 - 8.3. |
|
Some conflicts to resolve here now |
There was a problem hiding this comment.
Pull Request Overview
This PR bumps CodeIgniter to version 4.6.0 by updating supported PHP versions in documentation and adding support for PHP 8.4 in the PHP linter workflow.
- Updated INSTALL.md to reflect PHP version support change from 8.1–8.3 to 8.1–8.4.
- Updated .github/workflows/php-linter.yml to include a new linting job for PHP 8.4.
Reviewed Changes
Copilot reviewed 5 out of 23 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| INSTALL.md | Updated PHP version support documentation. |
| .github/workflows/php-linter.yml | Added a job to run PHP linting for version 8.4. |
Files not reviewed (18)
- app/Config/Cache.php: Language not supported
- app/Config/Constants.php: Language not supported
- app/Config/Database.php: Language not supported
- app/Config/Events.php: Language not supported
- app/Config/Feature.php: Language not supported
- app/Config/Format.php: Language not supported
- app/Config/Kint.php: Language not supported
- app/Config/Security.php: Language not supported
- app/Controllers/BaseController.php: Language not supported
- app/Views/errors/cli/error_exception.php: Language not supported
- app/Views/errors/html/debug.css: Language not supported
- app/Views/errors/html/error_400.php: Language not supported
- app/Views/errors/html/error_exception.php: Language not supported
- composer.json: Language not supported
- preload.php: Language not supported
- public/index.php: Language not supported
- public/license/LICENSE: Language not supported
- public/license/composer.LICENSES: Language not supported
There was a problem hiding this comment.
Pull Request Overview
This PR bumps the CodeIgniter CI version to 4.6.0 by updating relevant documentation and CI configuration. The key changes include:
- Updating the INSTALL.md file to reflect support for PHP 8.4.
- Adding a new PHP Lint step for PHP 8.4 in the GitHub Actions workflow.
Reviewed Changes
Copilot reviewed 5 out of 23 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| INSTALL.md | Updated supported PHP version to 8.4. |
| .github/workflows/php-linter.yml | Added a new PHP Lint step for PHP 8.4. |
Files not reviewed (18)
- app/Config/Cache.php: Language not supported
- app/Config/Constants.php: Language not supported
- app/Config/Database.php: Language not supported
- app/Config/Events.php: Language not supported
- app/Config/Feature.php: Language not supported
- app/Config/Format.php: Language not supported
- app/Config/Kint.php: Language not supported
- app/Config/Security.php: Language not supported
- app/Controllers/BaseController.php: Language not supported
- app/Views/errors/cli/error_exception.php: Language not supported
- app/Views/errors/html/debug.css: Language not supported
- app/Views/errors/html/error_400.php: Language not supported
- app/Views/errors/html/error_exception.php: Language not supported
- composer.json: Language not supported
- preload.php: Language not supported
- public/index.php: Language not supported
- public/license/LICENSE: Language not supported
- public/license/composer.LICENSES: Language not supported
- Revert PHP 8.4 support for now. - Removed extra space before comma
|
@jekkos I resolved the conflicts and made a minor change. This PR should be ready for review. It's now no longer showing so many file changes. |
|
Can you get rid of the merge commits in this branch? I'd like to keep master history clean. |
|
Does this PR now introduce support for 8.4? The linter is not completely happy yet. Also cool feature this copilot review. |
@jekkos I think you are referring to these two commits in this branch Those were generated by github when I resolved conflicts resulting from merging the two PRs generated by dependabot to bump versions. It won't let me merge the branch without resolving the conflicts related to that and a few other files, so I think I need some commit in there, though the naming is not helpful. Our standard procedure is to Squash and Merge. Before Merging it allows me to edit the commit message and I could replace the wording with more descriptive wording. Is that what you mean by getting rid of the merge commits to keep commit history clean? The only other thing I can think of is to delete the commits then rebase, but my understanding is that when you have multiple people making commits to the same branch, you should merge rather than rebase. Let me know if editing the commit message during the squash is acceptable or not. |
CodeIgniter 4.6.0 is PHP 8.4 compatible (https://codeigniter.com/user_guide/intro/requirements.html) but if you look at the linter, all the errors are deprecation warnings, so the problem is with the code, not CI. We either need to tell PHP Lint to report but not fail on deprecation warnings or turn off the 8.4 linter until we update the code. Either way I think upgrading the code to 8.4 compliance should be a separate PR than this one. I created issue #4200 |
|
@jekkos what do you want to do here? Can we turn off the PHP 8.4 linter or allow deprecation warnings but not errors? |
|
OK, for now I'm just going to merge so I can start on the events branch. We can create another PR to get rid of the deprecation warnings. I created #4200 for that. |
bump CodeIgniter to 4.6.0