Polish #11143
Polish #11143
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| /** | ||
| * An example attribute that requires a {@link Converter}. | ||
| * An example attribute that requires a {@link org.springframework.core.convert.converter.Converter}. |
There was a problem hiding this comment.
Can you expand why you think this is a good idea?
There was a problem hiding this comment.
@snicoll I'm not sure this is generally acceptable or just my preference but the rationale behind the change is that when an imported class is referenced only in Javadoc, to prevent it from cluttering import section, I usually prefer to use FQN in Javadoc instead of import.
There was a problem hiding this comment.
That sounds totally reasonable but that's not what we do here.
There was a problem hiding this comment.
@snicoll Sorry but I don't get it. Could you give me some more pointer for the last comment?
There was a problem hiding this comment.
@snicoll I guess you meant you want to revert the change, right?
There was a problem hiding this comment.
I do. Here is an example that we use quite a lot.
Having said that, I've seen that is not consistent, see AopAutoConfiguration for instance. I suggest to revert for now and I'll raise that at the next team call to check with the rest of the team.
| @@ -43,7 +43,7 @@ | |||
| * <p> | |||
| * Using this annotation will disable full auto-configuration and instead apply only | |||
| * configuration relevant to WebFlux tests (i.e. {@code @Controller}, | |||
| * {@code @ControllerAdvice}, {@code @JsonComponent}, {@code Converter}, and | |||
| * {@code @ControllerAdvice}, {@code @JsonComponent}, {@code Converter}, {@code GenericConverter}, and | |||
There was a problem hiding this comment.
I initially wrote that and I removed it as I felt it was a bit of a noise. So yeah if we scan a "converter" that includes the GenericConverter companion interface. Perhaps there is a better way to phrase that?
There was a problem hiding this comment.
@snicoll The reason why I added {@code GenericConverter} was that {@code Converter} is decorated with @code and as a code it didn't look to include GenericConverter. If it was "converter", it would be okay. I'm not sure how it can be improved ATM but maybe {@code Converter}/{@code GenericConverter}?
There was a problem hiding this comment.
Yeah I like that. Can you please update your proposal?
There was a problem hiding this comment.
@snicoll Sure, will do.
| @@ -32,13 +32,13 @@ | |||
|
|
|||
| @GetMapping("/two") | |||
| @ResponseBody | |||
| public String one(ExampleArgument argument) { | |||
| public String two(ExampleArgument argument) { | |||
snicoll
added
the
status: waiting-for-feedback
izeye
force-pushed
the
polish-20171125
branch
from
20eb30e to
bcda782
Compare
|
@snicoll Thanks for reviewing this. I changed as discussed. |
snicoll
added
priority: normal
type: task
* pr/11143: Polish contribution Polish
This PR fixes some typos and polishes trivial stuff.