Skip to content

Certificate Status Monitor Refactor #4103

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
Merged
merged 4 commits into from
Mar 18, 2025
Merged

Certificate Status Monitor Refactor #4103

merged 4 commits into from
Mar 18, 2025

Conversation

ArrisLee
Copy link
Collaborator

Which issue this PR addresses:

https://issues.redhat.com/browse/ARO-13644

What this PR does / why we need it:

  • Resolve some logic bugs in the original code.
  • Improve the readability
  • Add more error handling and edge case checking

Test plan for issue:

  • Unit test
  • CI/CD

@ArrisLee ArrisLee added skippy pull requests raised by member of Team Skippy ready-for-review labels Feb 16, 2025
@ArrisLee
Copy link
Collaborator Author

/azp run ci

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ArrisLee ArrisLee changed the base branch from master to yjst2012/hotfix-dns-renaming February 25, 2025 06:35
@ArrisLee ArrisLee changed the base branch from yjst2012/hotfix-dns-renaming to master February 25, 2025 06:36
@ArrisLee ArrisLee force-pushed the arris/refactor/certstatusmon branch from f2966e1 to 0453877 Compare February 25, 2025 07:00
@ArrisLee ArrisLee closed this Feb 25, 2025
@ArrisLee ArrisLee force-pushed the arris/refactor/certstatusmon branch from 2b0a9f9 to 796738b Compare February 25, 2025 07:14
@ArrisLee ArrisLee reopened this Feb 25, 2025
edisonLcardenas
edisonLcardenas previously approved these changes Mar 3, 2025
View reviewed changes
kimorris27
kimorris27 requested changes Mar 4, 2025
View reviewed changes
Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

It's a good refactor overall! The changes I'm suggesting are mostly regarding readability and potential for further refactoring.

@wanghaoran1988 wanghaoran1988 self-assigned this Mar 5, 2025
@ArrisLee ArrisLee requested a review from kimorris27 March 14, 2025 07:17
@wanghaoran1988 wanghaoran1988 merged commit 030b119 into master Mar 18, 2025
22 checks passed
s-fairchild pushed a commit that referenced this pull request Mar 19, 2025
@ArrisLee @s-fairchild
Certificate Status Monitor Refactor (#4103)
4d0ec09 
* rebase & updates

* remove redundant error fmt, reuse processCertificate func

* lint fix

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

@wanghaoran1988 wanghaoran1988 wanghaoran1988 approved these changes

@kimorris27 kimorris27 kimorris27 approved these changes

@edisonLcardenas edisonLcardenas edisonLcardenas left review comments

@jewzaam jewzaam Awaiting requested review from jewzaam

@bennerv bennerv Awaiting requested review from bennerv bennerv is a code owner

@hawkowl hawkowl Awaiting requested review from hawkowl hawkowl is a code owner

@rogbas rogbas Awaiting requested review from rogbas rogbas is a code owner

@petrkotas petrkotas Awaiting requested review from petrkotas

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@cblecker cblecker Awaiting requested review from cblecker cblecker is a code owner

@cadenmarchese cadenmarchese Awaiting requested review from cadenmarchese cadenmarchese is a code owner

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@SudoBrendan SudoBrendan Awaiting requested review from SudoBrendan

@yjst2012 yjst2012 Awaiting requested review from yjst2012 yjst2012 is a code owner

@jaitaiwan jaitaiwan Awaiting requested review from jaitaiwan jaitaiwan is a code owner

@anshulvermapatel anshulvermapatel Awaiting requested review from anshulvermapatel

@hlipsig hlipsig Awaiting requested review from hlipsig hlipsig is a code owner

@tiguelu tiguelu Awaiting requested review from tiguelu tiguelu is a code owner

@mociarain mociarain Awaiting requested review from mociarain mociarain is a code owner

@tsatam tsatam Awaiting requested review from tsatam tsatam is a code owner

@bitoku bitoku Awaiting requested review from bitoku

@fahlmant fahlmant Awaiting requested review from fahlmant fahlmant is a code owner

@sankur-codes sankur-codes Awaiting requested review from sankur-codes sankur-codes is a code owner

Assignees

@wanghaoran1988 wanghaoran1988

Labels
ready-for-review skippy pull requests raised by member of Team Skippy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ArrisLee @wanghaoran1988 @kimorris27 @edisonLcardenas