phoenixnap / springmvc-raml-plugin

Spring MVC - RAML Spec Synchroniser Plugin. A Maven plugin designed to Generate Server & Client code in Spring from a RAML API descriptor and conversely, a RAML API document from the SpringMVC Server implementation.
Apache License 2.0
136 stars 84 forks source link

ISSUE-212 Improving method naming based on unique reference name instead of hardcoded ById #213

Closed yuranos closed 6 years ago

yuranos commented 6 years ago

FYI: @stojsavljevic See #212 for details.

yuranos commented 6 years ago

@stojsavljevic , I found some parts of your commit related to #215 dubious. Let me come up with some corrections and we will review them together.

UPDATE: Never mind, mate. I was trying to improve "{" and "}" checks, but I found your code quite reasonable in the end. I wasn't well familiar with optional URI params.

yuranos commented 6 years ago

@stojsavljevic , are we merging this one?

stojsavljevic commented 6 years ago

Please check my comment: https://github.com/phoenixnap/springmvc-raml-plugin/issues/212#issuecomment-351743153

yuranos commented 6 years ago

Sorry, missed it. Checking it now...

stojsavljevic commented 6 years ago

Can you please revert your changes in older raml test files - issue-175.raml and issue-177.raml? Existing raml files are created to reproduce some resolved issue so we shouldn't change them. We can modify what is expected to be generated (IssueXXX.java.txt) but we must be careful so we don't re-introduce the issue. If you want to cover more use cases for the issue you are resolving - add another issue-212-X.raml.

yuranos commented 6 years ago

@stojsavljevic , done. It's not a problem for me to add more tests, but 3 cases per issue should be fine, I think. I tested all the cases I was thinking about.