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.

Sign up for GitHub

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

Jump to bottom

Provide PHP 8.2 support #5052

Closed
wants to merge 6 commits into from
Closed

Provide PHP 8.2 support #5052

wants to merge 6 commits into from

Conversation

raviks789
Copy link
Contributor

@raviks789 raviks789 commented Jul 3, 2023 •
edited by sukhwinder33445
Loading

Blocked by #5048

Depends on Icinga/icinga-php-thirdparty#11

@cla-bot cla-bot bot added the cla/signed label Jul 3, 2023
@raviks789 raviks789 changed the base branch from master to remove-vendor-dependency July 3, 2023 11:11
@raviks789 raviks789 force-pushed the provide-php82-support branch 3 times, most recently from e09ac6f to e5dd46e Compare July 4, 2023 08:00
@raviks789 raviks789 changed the title WIP Provide PHP 8.2 support Jul 4, 2023
@raviks789 raviks789 requested a review from lippserd July 4, 2023 08:23
@raviks789 raviks789 requested a review from nilmerg July 4, 2023 08:26
@raviks789 raviks789 marked this pull request as ready for review July 4, 2023 08:26
@raviks789 raviks789 force-pushed the provide-php82-support branch 10 times, most recently from d45d0be to eb46a57 Compare July 10, 2023 08:01
lippserd
lippserd requested changes Jul 10, 2023
View reviewed changes
library/Icinga/Application/Modules/Module.php Outdated Show resolved Hide resolved
library/Icinga/Util/LessParser.php Outdated Show resolved Hide resolved
@raviks789 raviks789 force-pushed the provide-php82-support branch 2 times, most recently from 9934bd4 to c90ea7a Compare July 10, 2023 11:59
@lippserd lippserd self-requested a review July 13, 2023 08:36
@lippserd
Copy link
Member

@raviks789 Please rebase.

@lippserd lippserd self-requested a review July 19, 2023 11:10
lippserd
lippserd requested changes Jul 19, 2023
View reviewed changes
Copy link
Member

@lippserd lippserd left a comment

Choose a reason for hiding this comment

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

localhost_icingaweb2_layout_menu

@lippserd
Copy link
Member

localhost_icingaweb2_layout_menu

@raviks789 @sukhwinder33445 Is this detected/fixed somewhere by PHPStan, etc?

@raviks789 raviks789 force-pushed the provide-php82-support branch 2 times, most recently from 0b65cc7 to 7ba8c6d Compare July 19, 2023 11:16
@raviks789
Copy link
Contributor Author

raviks789 commented Jul 19, 2023 •
edited
Loading

localhost_icingaweb2_layout_menu

@raviks789 @sukhwinder33445 Is this detected/fixed somewhere by PHPStan, etc?

The dynamic properties seems to be identified in #5040 by PHPStan. A few of them have been fixed by @sukhwinder33445 in the same PR. Should I move those related commits to this PR?

@sukhwinder33445
Copy link
Contributor

localhost_icingaweb2_layout_menu

@raviks789 @sukhwinder33445 Is this detected/fixed somewhere by PHPStan, etc?

Yes, detected and fixed here.

@sukhwinder33445
Copy link
Contributor

Screenshot 2023-07-19 at 15 54 43

Reproduce

Call null->getMyBug(); in any controller action and visit the url.

@nilmerg nilmerg force-pushed the remove-vendor-dependency branch 4 times, most recently from 70e38f0 to 0a6db98 Compare August 9, 2023 13:54
Base automatically changed from remove-vendor-dependency to master August 9, 2023 13:59
@nilmerg nilmerg force-pushed the provide-php82-support branch 3 times, most recently from e5663db to bf74a0b Compare August 10, 2023 12:13
raviks789 and others added 6 commits August 15, 2023 12:30
@raviks789
Add PHP 8.2 to the php workflow
491d202
@Al2Klimov @raviks789
Explicitly add Module#configdir as (non-dynamic) property
a7377d1 
W/o this the login page, if on PHP 8.2, says:

Deprecated: Creation of dynamic property Icinga\Application\Modules\Module::$configdir is deprecated in /usr/share/icingaweb2/library/Icinga/Application/Modules/Module.php on line 264

This breaks modules (if any) which use the property and not the getter.
@raviks789
Avoid use of dynamic properties deprecated in PHP 8.2
81c2f05
@raviks789
Remove dead code from Visitor.php
f36ba61 
The method `Visitor::visitColor` seems like a dead code and contains dynamic property
allocation. Hence, this method has been removed.
@raviks789
Fix parameter name mismatch in View::escape method
9cc8fbc 
The mismatch arises after migration from ZF1 to ZF-future.
@raviks789
Remove the use of unavailable property $this->lessc->importDisabled
8ffdbf8 
The functionality to set import to disabled is removed in `lessc`. Hence the code where this
property is used has been removed from `LessCompiler`.
@raviks789 raviks789 mentioned this pull request Aug 16, 2023
Merged
@sukhwinder33445
Copy link
Contributor

#5040 contain these commits.

@sukhwinder33445 sukhwinder33445 deleted the provide-php82-support branch August 21, 2023 14:21
nilmerg added a commit that referenced this pull request Aug 23, 2023
@nilmerg
Github Actions: Add PhpStan (#5040)
6c68578 
Blocked by: #5061 
closes #5052, #4920,
#5003
@nilmerg nilmerg removed their request for review August 23, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lippserd lippserd lippserd requested changes

Assignees
No one assigned
Labels
cla/signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@raviks789 @lippserd @sukhwinder33445 @Al2Klimov