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.

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

Fix Issue #28 #29

Closed
wants to merge 26 commits into from
Closed

Fix Issue #28 #29

wants to merge 26 commits into from

Conversation

seungsoo-lee
Copy link
Collaborator

Description

Fixes #28

Does this PR introduce a breaking change?

Checklist

  • PR title follows the <type>: <description> convention
  • I use conventional commits in my commit messages
  • I have updated the documentation accordingly
  • I Keep It Small and Simple: The smaller the PR is, the easier it is to review and have it merged
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Additional information for reviewer

Mention if this PR is part of any design or a continuation of previous PRs

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the specific purpose of this script in explicitly removing the CRs? Under what circumstances should a user execute it?

b0m313 and others added 3 commits January 5, 2024 19:02
- Fixed code so that when deleting a SIB, logs are properly generated and the created nimberpolicy is also deleted.
@anurag-rajawat
Copy link
Collaborator

Please squash commits.

Copy link
Collaborator

@rajaSahil rajaSahil left a comment

Choose a reason for hiding this comment

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

@seungsoo-lee PR looks good, please squash the commits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need these sophisticated parameters in SecurityIntent; if there is a need for parameters, we'll specify those generically.

@anurag-rajawat
Copy link
Collaborator

anurag-rajawat commented Jan 9, 2024

@seungsoo-lee, we can close this PR since the changes have been split into two separate PRs for better organization and review:
#30 resolves the issues addressed in #28.
#31 introduces the new features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Deletion of SIB results in crash
4 participants