Skip to content

Comments

Avoid use of String's for ContentDisposition type#25647

Closed
kashike wants to merge 1 commit intospring-projects:masterfrom
kashike:feature/content-disposition-constants
Closed

Avoid use of String's for ContentDisposition type#25647
kashike wants to merge 1 commit intospring-projects:masterfrom
kashike:feature/content-disposition-constants

Conversation

@kashike
Copy link
Contributor

@kashike kashike commented Aug 26, 2020

This adds constants for three common Content-Disposition types: attachment, form-data, and inline.

I have also updated the tests to use these new constants.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 26, 2020
@kashike
Copy link
Contributor Author

kashike commented Oct 2, 2020

Is there anything holding this pull request back? It is very simple and non-breaking.

@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Oct 15, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 15, 2020 •
edited
Loading

I think adding alternatives to the static builder(String) such as formData(), inline(), and attachment() would make the case of building more convenient than using constants and that be good enough since I'm guessing that building is the more common case.

WDYT and if you agree would you like to update your PR with those changes?

@rstoyanchev rstoyanchev self-assigned this Oct 15, 2020
@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Oct 15, 2020
@kashike
Copy link
Contributor Author

kashike commented Oct 16, 2020

@rstoyanchev I have added the helper methods, but I have also kept the new constants as well, as they are still useful - for example, checking the type: disposition.getType().equals(ContentDisposition.ATTACHMENT).

Let me know if this is okay with you now.

@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 Oct 16, 2020
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 22, 2020
@rstoyanchev rstoyanchev added this to the 5.3 GA milestone Oct 22, 2020
rstoyanchev
rstoyanchev requested changes Oct 22, 2020
View reviewed changes
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

Thanks I've schedule it for 5.3.

@kashike
Copy link
Contributor Author

kashike commented Oct 22, 2020

@rstoyanchev How does that look now?

@rstoyanchev rstoyanchev changed the title Provide common Content-Disposition types as constants Avoid use of String's for ContentDisposition type Oct 23, 2020
rstoyanchev pushed a commit that referenced this pull request Oct 23, 2020
@kashike @rstoyanchev
Improve support for Content-Disposition types
6e640c8 
See gh-25647
@sync-by-unito sync-by-unito bot mentioned this pull request Oct 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rstoyanchev rstoyanchev rstoyanchev requested changes

Assignees

@rstoyanchev rstoyanchev

Labels

in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement

Projects

None yet

Milestone

5.3 GA

Development

Successfully merging this pull request may close these issues.

3 participants

@kashike @rstoyanchev @spring-projects-issues