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

fix: use status instead of event for pdb checking #477

Merged
merged 1 commit into from
Jun 7, 2023
Merged

fix: use status instead of event for pdb checking #477

merged 1 commit into from
Jun 7, 2023

Conversation

jianzhangbjz
Copy link
Contributor

Closes #476

📑 Description

The PDB event is a historical record, it's not a good choice to judge pdb status by based on it. So, I used the status instead of event in this PR.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

@jianzhangbjz jianzhangbjz requested review from a team as code owners June 2, 2023 09:07
AlexsJones
AlexsJones reviewed Jun 2, 2023
View reviewed changes
pkg/analyzer/pdb.go Outdated Show resolved Hide resolved
arbreezy
arbreezy requested changes Jun 4, 2023
View reviewed changes
Copy link
Member

@arbreezy arbreezy left a comment

Choose a reason for hiding this comment

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

Update your commit message as well, to conform with release-please

@jianzhangbjz jianzhangbjz changed the title use status instead of event to check pdb fix: use status instead of event for pdb checking Jun 5, 2023
AlexsJones
AlexsJones approved these changes Jun 5, 2023
View reviewed changes
Copy link
Member

@AlexsJones AlexsJones left a comment

Choose a reason for hiding this comment

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

LGTM, but as @arbreezy says please update commit message

@jianzhangbjz
Copy link
Contributor Author

please update commit message

Hi @AlexsJones @arbreezy , thanks for your review. I will squash those two commits, but, sorry, what commit message I should add? Thanks!

@jianzhangbjz
fix: use the status for pdb checking
fe04676 
The PDB event is a historical record, it's not a good choice to judge the pdb latest status based on it.
So, use the `stataus` instead of `event` to check it.

Closes: k8sgpt-ai#476

Signed-off-by: Jian Zhang <jiazha@redhat.com>
@jianzhangbjz
Copy link
Contributor Author

Hi @AlexsJones @AlexsJones , I updated the commit message, please let me know if any thing else is needed, thanks!

    fix: use the `status` for pdb checking
    
    The PDB event is a historical record, it's not a good choice to judge the pdb latest status based on it.
    So, use the `stataus` instead of `event` to check it.
    
    Closes: #476
    
    Signed-off-by: Jian Zhang <jiazha@redhat.com>

@jianzhangbjz
Copy link
Contributor Author

jianzhangbjz commented Jun 7, 2023 •
edited
Loading

Hi @AlexsJones @arbreezy, seems like the merging is blocked by The base branch restricts merging to authorized users.. Could you help have a look? Thanks! Is there something my end I must do to be "authorized"?
image

@AlexsJones AlexsJones merged commit 075066d into k8sgpt-ai:main Jun 7, 2023
@github-actions github-actions bot mentioned this pull request Jun 7, 2023
Merged
@jianzhangbjz jianzhangbjz deleted the pdb branch June 7, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arbreezy arbreezy arbreezy approved these changes

@AlexsJones AlexsJones AlexsJones approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Backlog
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Question]: should not use the history event for the pdb checking
3 participants
@jianzhangbjz @AlexsJones @arbreezy