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

Adopting Security Groups API V3 #1185

Closed
wants to merge 21 commits into from

Conversation

radoslav-tomov
Copy link
Contributor

@radoslav-tomov radoslav-tomov commented Jul 7, 2023

Adopting Security Groups API V3 API. Change includes implementation, unit and integration tests for the following operations:

PR ended up a bit on the large side, but it is mostly boilerplate, so I decided not to try to brake it further apart.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 7, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@radoslav-tomov radoslav-tomov marked this pull request as ready for review July 19, 2023 12:13
@radoslav-tomov radoslav-tomov changed the title [WIP]Adopting Security Groups API V3 Adopting Security Groups API V3 Jul 19, 2023
Copy link
Contributor

@anthonydahanne anthonydahanne left a comment

Choose a reason for hiding this comment

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

all tests pass; I've had a quick look, please fix obvious issues (cf screenshots)

thank you
Screenshot 2023-10-04 at 17 34 26
Screenshot 2023-10-04 at 17 34 13

@radoslav-tomov
Copy link
Contributor Author

@anthonydahanne I've removed the unused imports and reformatted via the standard google eclipse style formatter. Fixed the year in the license comment and added it where it was missing. Unfortunately I don't have InteliJ license, so I might have missed some warnings. Let me know if this is the case.

@anthonydahanne
Copy link
Contributor

Hello @radoslav-tomov !
It's better, thank you, but still a lot of useless reformating.

But I don't blame you at all; the project should have a linter in place; working on that!

pivotal-david-osullivan added a commit that referenced this pull request Feb 12, 2024
Migrate integration tests to Junit 5 and selectively apply changes from  #1174 and #1185
@anthonydahanne
Copy link
Contributor

was included and merged with this PR #1215

@pivotal-david-osullivan
Copy link
Contributor

Closing as this work was included in #1215

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.

3 participants