Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add missing attributes to ServletRegistration annotation #45007

Closed
wants to merge 9 commits into from
Closed

Add missing attributes to ServletRegistration annotation #45007

wants to merge 9 commits into from

Conversation

dmytrodanilenkov
Copy link
Contributor

Closes #45001

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 4, 2025
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
12bf433 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@mhalbritter
Copy link
Contributor

I like the handling of initParameters and servletRegistrationBeans in #45005. Could you rework your PR to match #45005?

@mhalbritter mhalbritter mentioned this pull request Apr 7, 2025
Closed
@mhalbritter mhalbritter changed the title add properties initParameters servletRegistrationBeans multipartConfig to @ServletRegistration Add missing attributes to ServletRegistration annotation Apr 7, 2025
@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Apr 7, 2025
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
da9173b 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Copy link
Contributor Author

there’s no built-in servletRegistrationBeans property or method in Spring Boot’s standard ServletRegistrationBean. for now I removed it. or maybe I understand issue incorrectly?

@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
3ca2d29 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 7, 2025
@mhalbritter
Copy link
Contributor

Sorry, my mistake. There's no setServletRegistrationBeans on ServletRegistrationBean, it's only on FilterRegistrationBean.

@mhalbritter mhalbritter added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Apr 8, 2025
@mhalbritter mhalbritter added this to the 3.5.x milestone Apr 8, 2025
@mhalbritter mhalbritter mentioned this pull request Apr 8, 2025
Closed
dmytrodanilenkov and others added 3 commits April 8, 2025 10:06
@dmytrodanilenkov
Merge branch 'main' into fix-issue-45001
fce9e44 
Signed-off-by: Dmytro Danilenkov <63580205+dmytrodanilenkov@users.noreply.github.com>
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
839b087 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
9bb90c9 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Copy link
Contributor Author

so changes done, please review

nosan
nosan reviewed Apr 9, 2025
View reviewed changes
Copy link
Contributor

@nosan nosan left a comment

Choose a reason for hiding this comment

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

Thanks, @dmytrodanilenkov!
I've left some comments for your consideration.

Could you also take a look at the Checkstyle issues?

You can verify them using:

./gradlew checkstyleMain checkstyleTest

@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
c0a8409 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@nosan
Copy link
Contributor

nosan commented Apr 9, 2025 •
edited
Loading

Thanks, @dmytrodanilenkov

Execution failed for task ':spring-boot-project:spring-boot:checkFormatMain'.
> Formatting violations found in the following files:
   * src/main/java/org/springframework/boot/web/servlet/ServletContextInitializerBeans.java

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

Could you please run ./gradlew format?

Additionally, I've noticed some minor changes unrelated to your PR, such as removed blank lines. Could you please revert these?

To verify your changes: ./gradlew :spring-boot-project:spring-boot:build or ./gradlew build

@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
d4bdaa1 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
4c1af47 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Copy link
Contributor Author

@nosan done

@mhalbritter mhalbritter self-assigned this Apr 10, 2025
mhalbritter pushed a commit that referenced this pull request Apr 10, 2025
@dmytrodanilenkov @mhalbritter
Add missing attributes to ServletRegistration annotation
d012807 
See gh-45007

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
mhalbritter added a commit that referenced this pull request Apr 10, 2025
@mhalbritter
Polish "Add missing attributes to ServletRegistration annotation"
5401327 
See gh-45007
@mhalbritter
Copy link
Contributor

Thanks @dmytrodanilenkov ! And thanks @nosan for the review!

@mhalbritter mhalbritter modified the milestones: 3.5.x, 3.5.0-RC1 Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nosan nosan nosan left review comments

@mhalbritter mhalbritter mhalbritter left review comments

Assignees

@mhalbritter mhalbritter

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
3.5.0-RC1
Development

Successfully merging this pull request may close these issues.

Add missing attributes to ServletRegistration annotation
4 participants
@dmytrodanilenkov @mhalbritter @nosan @spring-projects-issues