Skip to content
This repository was archived by the owner on May 14, 2025. It is now read-only.

Fix rest documentation for changes to spring-rest-docs #6012

Merged

Conversation

corneil
Copy link
Contributor

@corneil corneil commented Oct 24, 2024

Update request-parameters to query-parameters
Added optional where applicable
Added snippet template to query-parameters and updated template for path-parameters to improve readability.

Fixes #6008

Update request-parameters to query-parameters
Added optional where applicable
Added snippet template to query-parameters and updated template for path-parameters to improve readability.

Fixes spring-attic#6008
@corneil corneil requested review from cppwfs and onobc October 24, 2024 15:09
Copy link
Contributor

@onobc onobc left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement @corneil . Nice work!

@@ -233,15 +233,18 @@ void listTaskExecutions() throws Exception {

this.mockMvc.perform(
get("/tasks/executions")
.queryParam("page", "1")
.queryParam("size", "2"))
.queryParam("page", "0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious why the change in page and size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To ensure result has data we needed page=0.
I thought size=10 is a good consistent value across usages.

.queryParam("page", "0")
.queryParam("size", "10")
.queryParam("sort", "END_TIME,desc")
)
.andDo(print())
Copy link
Contributor

@cppwfs cppwfs Oct 24, 2024

Choose a reason for hiding this comment

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

nitpick: We can remove the print() statement. Let's save those for debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed print from all documentation classes.

parameterWithName("size")
.description("The requested page size (optional)")),
.description("The requested page size")),
Copy link
Contributor

Choose a reason for hiding this comment

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

We are missing the .optional() method here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Fixed

@cppwfs
Copy link
Contributor

cppwfs commented Oct 24, 2024

@corneil Looks really good. Just a one small correction, one question, and a nitpick

Corneil du Plessis added 2 commits October 25, 2024 13:09
@cppwfs
Copy link
Contributor

cppwfs commented Oct 25, 2024

@corneil thanks for the good work and investigation on this!

@corneil corneil requested review from cppwfs and onobc October 25, 2024 12:43
Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

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

@corneil thanks for the good work and investigation on this!

@corneil corneil merged commit d2a77ef into spring-attic:main Oct 28, 2024
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Change RequestParam to QueryParam for in api-guide.adoc
3 participants