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

Fixed uninitialized value warning in AdminSystemMaintenance. #298

Merged
merged 8 commits into from
Jan 20, 2023
Merged

Fixed uninitialized value warning in AdminSystemMaintenance. #298

merged 8 commits into from
Jan 20, 2023

Conversation

tipue-dev
Copy link
Contributor

Proposed change

Clicking on a link to edit a AdminSystemMaintenance-Entry logs a warning because there is no Notification parameter given yet Action=AdminSystemMaintenance;Subaction=SystemMaintenanceEdit;SystemMaintenanceID=1

[Thu Sep 29 17:29:39 2022] index.pl: Use of uninitialized value in string eq at /opt/frameworks/Znuny-Dev/bin/cgi-bin/../../Kernel/Modules/AdminSystemMaintenance.pm line 242.
[Thu Sep 29 17:29:39 2022] index.pl: Use of uninitialized value in string eq at /opt/frameworks/Znuny-Dev/bin/cgi-bin/../../Kernel/Modules/AdminSystemMaintenance.pm line 251.

Type of change

1 - 🐞 bug 🐞

Breaking change

Additional information

Checklist

  • The code change is tested and works locally.(❗)
  • There is no commented out code in this PR.(❕)
  • You improved or added new unit tests.(❕)
  • Local ZnunyCodePolicy run passes successfully.(❕)
  • Local unit tests pass.(❕)
  • GitHub workflow ZnunyCodePolicy passes.(❗)
  • GitHub workflow unit tests pass.(❗)

@dennykorsukewitz dennykorsukewitz self-assigned this Oct 26, 2022
@dennykorsukewitz dennykorsukewitz added 1 - 🐞 bug 🐞 An issue with the system. 4 - verified This issue or pull request was verified. 1 - 💎 code quality Code quality improvements to existing code or addition of unit tests. 3 - wait for reviewer Znuny, it's your turn. 2 - Frontend labels Oct 26, 2022
dennykorsukewitz
dennykorsukewitz requested changes Oct 26, 2022
View reviewed changes
Copy link
Member

@dennykorsukewitz dennykorsukewitz left a comment

Choose a reason for hiding this comment

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

Hi @tipue-dev ,
thanks for your PR.

I have a few change requests here.
We should grab the param only once, at the beginning of the function

Also check these lines:
https://github.com/znuny/Znuny/blob/dev/Kernel/Modules/AdminSystemMaintenance.pm#L487
https://github.com/znuny/Znuny/blob/dev/Kernel/Modules/AdminSystemMaintenance.pm#L492

Regards 🚀

Kernel/Modules/AdminSystemMaintenance.pm Outdated Show resolved Hide resolved
Kernel/Modules/AdminSystemMaintenance.pm Outdated Show resolved Hide resolved
@dennykorsukewitz dennykorsukewitz added 3 - wait for pipeline Wait for green GitHub pipeline. and removed 3 - wait for reviewer Znuny, it's your turn. labels Jan 10, 2023
@dennykorsukewitz dennykorsukewitz added this to the rel-6_5_1 milestone Jan 10, 2023
@dennykorsukewitz dennykorsukewitz added 3 - wait for merge Znuny, it's your turn. and removed 3 - wait for pipeline Wait for green GitHub pipeline. labels Jan 20, 2023
@dennykorsukewitz dennykorsukewitz changed the title Fixed uninitialized value warning in AdminSystemMaintenance Fixed uninitialized value warning in AdminSystemMaintenance. Jan 20, 2023
@dennykorsukewitz dennykorsukewitz merged commit 5f170d1 into znuny:dev Jan 20, 2023
@dennykorsukewitz dennykorsukewitz removed the 3 - wait for merge Znuny, it's your turn. label Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dennykorsukewitz dennykorsukewitz dennykorsukewitz approved these changes

Assignees

@dennykorsukewitz dennykorsukewitz

Labels
1 - 🐞 bug 🐞 An issue with the system. 1 - 💎 code quality Code quality improvements to existing code or addition of unit tests. 2 - Frontend 4 - verified This issue or pull request was verified.
Milestone
rel-6_5_1
Development

Successfully merging this pull request may close these issues.
2 participants
@tipue-dev @dennykorsukewitz