Skip to content

Conversation

@jrauh01
Copy link
Contributor

@jrauh01 jrauh01 commented Nov 13, 2025

As we now use php 8.2 as minimal version, the following modernizations have been added to the code:

  • Typed class properties
  • Return values for methods
  • Match expressions
  • Modern string functions (e.g. str_contains(), str_starts_with() and str_ends_with())
  • Nullsafe operator usage
  • Deprecated code has been replaced
  • Imports are now used everywhere

Additionally the import blocks have been sorted, indentation has been fixed, and some smaller adjustments.

resolve #378
require Icinga/ipl-web/pull/342

@jrauh01 jrauh01 self-assigned this Nov 13, 2025
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Nov 13, 2025
@jrauh01 jrauh01 added the enhancement New feature or improvement label Nov 13, 2025
@jrauh01 jrauh01 force-pushed the use-typed-class-attributes branch 3 times, most recently from 2bf4959 to 25c7cb4 Compare November 14, 2025 09:29
@jrauh01 jrauh01 requested a review from nilmerg November 14, 2025 09:34
@jrauh01 jrauh01 force-pushed the use-typed-class-attributes branch 3 times, most recently from 34029a8 to 3af0375 Compare November 18, 2025 08:16
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 left a comment

Choose a reason for hiding this comment

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

I have looked through the files up to MoveRotationForm.php.

@jrauh01 jrauh01 force-pushed the use-typed-class-attributes branch from 3af0375 to 3723f76 Compare November 24, 2025 14:52
@jrauh01 jrauh01 force-pushed the use-typed-class-attributes branch 2 times, most recently from 0094a30 to c043776 Compare November 25, 2025 13:09
@jrauh01 jrauh01 force-pushed the use-typed-class-attributes branch 3 times, most recently from 1fa2990 to 5820718 Compare November 26, 2025 10:10
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 left a comment

Choose a reason for hiding this comment

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

LGTM

@jrauh01 jrauh01 force-pushed the use-typed-class-attributes branch from 5820718 to 3cfc457 Compare November 27, 2025 06:07
@jrauh01
Copy link
Contributor Author

jrauh01 commented Nov 27, 2025

The last force push did not change anything, but only moved a change from one commit into another.

'24-7' => $rotation->options['at'],
'partial' => $rotation->options['from'],
'multi' => $rotation->options['from_at'],
default => throw new LogicException('Invalid mode')
Copy link
Member

Choose a reason for hiding this comment

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

This is about modernizing code. A logic exception is something that must not happen. It's simply a programming error and an end-user should never face it. As such, this is only to prevent the routine continue and to not run into a even more convoluted error.

So, back to modernizing, what does a match expression do without a default case? Right, complain.

It's as simple as that. What does the logic exception do? Complain. The same thing. So let's get rid of it already.

property: 'data',
ref: '#/components/schemas/' . $entityName,
description: sprintf('Successfull response with the %s object', $entityName),
description: sprintf('Successful response with the %s object', $entityName),
Copy link
Member

Choose a reason for hiding this comment

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

This has an influence on the rendered api description. Update it by running icingacli notifications openapi generate.

$this->object instanceof Event => new EventRenderer(),
$this->object instanceof Incident => new IncidentRenderer(),
$this->object instanceof Contactgroup => new ContactgroupRenderer(),
default => throw new NotImplementedError('Not implemented')
Copy link
Member

Choose a reason for hiding this comment

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

same here, let the match throw

@nilmerg nilmerg removed the enhancement New feature or improvement label Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed CLA is signed by all contributors of a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use typed class attributes

4 participants