-
Notifications
You must be signed in to change notification settings - Fork 31
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
Cleanup some sonarcloud reported issues #345
Cleanup some sonarcloud reported issues #345
Conversation
7bec769
to
acb0bcf
Compare
A good basis for further cleanup later as well - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqZxRJnw-QF9_Gfr - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqZxRJnw-QF9_Gfq - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqYTRJnw-QF9_Ge4 - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqWBRJnw-QF9_Gdg - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqV9RJnw-QF9_Gdd - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqV9RJnw-QF9_Gde - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqVoRJnw-QF9_GdT - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqXdRJnw-QF9_Gei - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqXuRJnw-QF9_Geq - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqYKRJnw-QF9_Ge0
acb0bcf
to
26e26aa
Compare
|
@@ -190,11 +190,11 @@ public UninstallService destroyApp( | |||
@Parameter(hidden = true) Region region, | |||
@Parameter(hidden = true) Project project, | |||
@RequestParam(value = "path", required = false) String path, | |||
@RequestParam(value = "bulk", required = false) boolean bulk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just remove this parameter, @olevitt ? Onyxia-web doesn't use it, e.g. for the 'delete all' onyxia-web send multiple delete calls (one for each service)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok to keep it as it may be useful for other clients (such as an admin tool that would cleanup)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks 👍
Draft PR for now, just for visibility that I am working on this one.
A good basis for further cleanup later as well