Changes for issue #23915.
Comment From: mikesmithson
is this issue something a first-time contributor can submit a PR for? I saw that it was self-assigned back in December and wasn't sure if it was being actively worked on. I have a PR that I can submit if this issue is still available for outside contribution.
The necessary changes are quite small . The tricky part is reviewing the Javadoc in all the relevant classes and making updates, especially with regards to defaults. If you see the classes touched by commits associated with #24179, that will give you an idea.
The reference documentation will also need to be checked and updated as needed.
I see that
RequestMappingInfohas a similar property that needs changing too.There might be more but this should give enough for another draft.
@rstoyanchev - OK, am I at least on the right track or do I need to regroup and start from the beginning?
Comment From: rstoyanchev
Yes that flag does need to change. My intention was to list what else would be needed.
Comment From: mikesmithson
Yes that flag does need to change. My intention was to list what else would be needed.
@rstoyanchev - OK, I pushed another change. Crossing my fingers...
Comment From: mikesmithson
@rstoyanchev - Is there any interest in this PR still?
Comment From: kuberr
@rstoyanchev - Is there any interest in this PR still?
Yes. We are facing a deprecation warning which I'm not sure if it will be solved by this PR. We currently use this:
pathMatchConfigurer.setUseSuffixPatternMatch(false);
and:
contentNegotiationConfigurer.favorPathExtension(false);
Bu it gives a deprecation warning. I was hoping this PR would allow us to remove this line of code because false would become the new default.
However, when I look at the actual changes in this PR I see that the useSuffixMatch variable is still set to true (alongside the useRegisteredSuffixMatch one). Shouldn't it be set to false by default?
Comment From: rstoyanchev
Is there any interest in this PR still?
@mikesmithson yes, the issue is scheduled for 5.3 M1 but master is not tracking 5.3.x yet. The PR is incomplete as it stands. See comment by @kuberr above and also the Javadoc and documentation will need a more thorough review and updates.
I'm sorry that I can't give more detailed guidance and there is a reason this is not marked as ideal for contribution. It is tricky with lots of small changes that require a lot of attention. You can see how much work it took to apply deprecations in #24179 and you can also use that for clues if you'd like to take this further.
Comment From: mikesmithson
@kuberr @rstoyanchev - I would gladly like to take this issue to the finish line if I can get incremental feedback along the way as I am mostly flying blind on a fair amount of this PR. I would probably have to make several small commits along the way and then make adjustments if necessary based on your feedback. Is that a reasonable proposal to you?
Comment From: rstoyanchev
@mikesmithson by all means, any further improvements would be welcome.
Comment From: rstoyanchev
@mikesmithson, I've resolved this with 147b8fb75550ebe25ebdae6a42af74c61b24439f. As I mentioned this really wasn't ideal for contribution. Thanks for the pull request in any case!