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

Allow original_filename to be used for all content types #314

Merged
merged 1 commit into from
Jun 30, 2022
Merged

Allow original_filename to be used for all content types #314

merged 1 commit into from
Jun 30, 2022

Conversation

schinery
Copy link
Contributor

Allow original_filename to be used for all content types not just StringIO, so that the generated Tempfile can potentially renamed when generated.

@schinery schinery mentioned this pull request Jun 30, 2022
Closed
jeremyevans
jeremyevans requested changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Looks mostly good, please see requested changes.

lib/rack/test/uploaded_file.rb Outdated Show resolved Hide resolved
lib/rack/test/uploaded_file.rb Outdated Show resolved Hide resolved
lib/rack/test/uploaded_file.rb Outdated Show resolved Hide resolved
lib/rack/test/uploaded_file.rb Outdated Show resolved Hide resolved
jeremyevans
jeremyevans approved these changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

ioquatix
ioquatix requested changes Jun 30, 2022
View reviewed changes
Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

LGTM just a minor suggested change.

@schinery schinery requested a review from ioquatix June 30, 2022 21:30
ioquatix
ioquatix approved these changes Jun 30, 2022
View reviewed changes
Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

Great work.

@ioquatix ioquatix enabled auto-merge (squash) June 30, 2022 21:31
@ioquatix ioquatix merged commit 2a053a2 into rack:main Jun 30, 2022
@schinery
Copy link
Contributor Author

schinery commented Jul 5, 2022

Any ideas when you might be able to push a release out with this change in? Only so I can get rid of some hacky code I threw in a project to workaround 😄

@jeremyevans
Copy link
Contributor

We are probably at least a few weeks out from a release, considering 2.0.0 was released less than 2 weeks ago.

@inkstak
Copy link

inkstak commented Mar 14, 2023

Last release has few months ago. Any plan to release this change ?

@ioquatix
Copy link
Member

@jeremyevans is there any reason why we can't release 2.1? or something else?

@jeremyevans
Copy link
Contributor

I can put out a new release this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremyevans jeremyevans jeremyevans approved these changes

@ioquatix ioquatix ioquatix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@schinery @jeremyevans @inkstak @ioquatix