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

feat(server): add DELETE API for Carapace certificates #475

Merged
merged 1 commit into from
Jun 6, 2024
Merged

feat(server): add DELETE API for Carapace certificates #475

merged 1 commit into from
Jun 6, 2024

Conversation

NiccoMlt
Copy link
Contributor

@NiccoMlt NiccoMlt commented Jun 4, 2024 •
edited
Loading

Closes #454

@NiccoMlt NiccoMlt linked an issue Jun 4, 2024 that may be closed by this pull request
Implement API Create/Delete and edit certificate #454
Closed
@NiccoMlt NiccoMlt requested a review from hamadodene June 4, 2024 16:02
@NiccoMlt NiccoMlt self-assigned this Jun 4, 2024
hamadodene
hamadodene approved these changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@hamadodene hamadodene left a comment

Choose a reason for hiding this comment

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

LGTM...
@NiccoMlt Can you check the test that fails? It could be one of the flacky tests. If you can fix it it wouldn't be bad.

@hamadodene
Copy link
Contributor

Are you planning to integrate it on the admin side too?
Otherwise, can you make another follow up issue to integrate it if it isn't already there?

@NiccoMlt
Copy link
Contributor Author

NiccoMlt commented Jun 5, 2024

Are you planning to integrate it on the admin side too? Otherwise, can you make another follow up issue to integrate it if it isn't already there?

The issue already exists, the UI support was not planned for this issue: #357

LGTM... @NiccoMlt Can you check the test that fails? It could be one of the flacky tests. If you can fix it it wouldn't be bad.

Ok, I'll give it a look

@NiccoMlt NiccoMlt mentioned this pull request Jun 5, 2024
Open
@NiccoMlt NiccoMlt force-pushed the 454-implement-api-createdelete-and-edit-certificate branch from e358b77 to bd8b8fd Compare June 5, 2024 12:23
@NiccoMlt NiccoMlt force-pushed the 454-implement-api-createdelete-and-edit-certificate branch from bd8b8fd to 64b301a Compare June 6, 2024 10:28
@NiccoMlt
Copy link
Contributor Author

NiccoMlt commented Jun 6, 2024

LGTM... @NiccoMlt Can you check the test that fails? It could be one of the flacky tests. If you can fix it it wouldn't be bad.

Ok, I'll give it a look

@hamadodene sorry but I didn't manage to reproduce it in any way, it's easier to break it for "too many files" than to have the same problem happen locally:

image

dmercuriali
dmercuriali approved these changes Jun 6, 2024
View reviewed changes
Copy link
Contributor

@dmercuriali dmercuriali left a comment

Choose a reason for hiding this comment

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

LGTM

@dmercuriali dmercuriali merged commit d52f7d9 into master Jun 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmercuriali dmercuriali dmercuriali approved these changes

@hamadodene hamadodene Awaiting requested review from hamadodene

Assignees

@NiccoMlt NiccoMlt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement API Create/Delete and edit certificate
3 participants
@NiccoMlt @hamadodene @dmercuriali