search

ScaCap / spring-auto-restdocs

Spring Auto REST Docs is an extension to Spring REST Docs
https://scacap.github.io/spring-auto-restdocs/
Apache License 2.0
310 stars 86 forks source link

#426 RequestParamSnippet now resolves DTOs #427

Closed mustaphazorgati closed 3 years ago

mustaphazorgati commented 3 years ago

This is just an idea to solve #426. I am not sure weather I like the switch from RequestParamSnippet -> RequestFieldSnippet. This is semantically false.

On the other side this is a quick fix for the issue.. Adding the requested functionallity can be complicated.

jmisur commented 3 years ago

Hey @mustaphazorgati thanks for contribution! The changes look good. If you could also add relevant unit tests, e.g. to FieldDocumentationGeneratorTest which you updated. Also it would be nice to have a practical example in ItemResource at least in mvc sample project.

mustaphazorgati commented 3 years ago

Hey @jmisur! Thanks for you answer. I was a little conserned because I didn't hear from you for about 2 weeks ^^. The semantic change from RequestParamSnippet to RequestFieldSnippet is fine with you? That's the reason I didn't write any tests. This feels kinda off because like in #426 mentioned we're trying to document query parameters.

mustaphazorgati commented 3 years ago

Or am I just too picky?

mustaphazorgati commented 3 years ago

Since I just had a deeper look during the debugging session for #423 I am now convinced that my proposed solution is not good. I have an idea, but for that I need a little input from your end. As mentioned in #423 I think a quick video call would be very helpful.

mustaphazorgati commented 3 years ago

@jmisur, can you please have a look at this? For us this is kind of urgent since we lose quiet a chunk of our generated documentation.

mustaphazorgati commented 3 years ago

Hey @jmisur, I just updated the PR with a solution I think works well.. Unfortunately there are 2 test failures. After having a look I think that those tests are questionable. I'd love to discuss this. Please answer soon.

jmisur commented 3 years ago

Thanks @mustaphazorgati for these contributions. I'll take a look at it this week and try to make it work.

mustaphazorgati commented 3 years ago

Hey @jmisur did you get a chance to look at this? Can I support you somehow?

jmisur commented 3 years ago

Sry extremely busy week. I'll try to look at it when I get a chance.

mustaphazorgati commented 3 years ago

No worries. Don't stress yourself.

mustaphazorgati commented 3 years ago

Hey mate! Can we get this done this year? :)

mustaphazorgati commented 3 years ago

I think when we have a quick chat (<30min) I have enough input to continue editing this. That'd be awesome and is hopefully not too much effort on your side @jmisur

mustaphazorgati commented 3 years ago

@jmisur bump :)