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.

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

Docs: Breaking change in 0.8.0 re: UploadedFile.new #209

Merged
merged 4 commits into from Nov 20, 2017

Conversation

jaredbeck
Copy link
Contributor

[ci skip]

History.md Outdated
* Breaking Change
* In `UploadedFile.new`, when passing a `Pathname`, an additional argument
is now required (`original_filename`). Provide the new argument or
call `to_s` on your `Pathname`.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the error you get if you don't do this? Is it the nil error we are seeing in #207?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error message is "Missing original_filename for IO". It comes from initialize_from_io.

      def initialize_from_io(io, original_filename)
        @tempfile = io
        @original_filename = original_filename || raise(ArgumentError, 'Missing `original_filename` for IO')
      end

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, interesting. There seems to be a couple of issues there, because people have also reported of nil error coming:

NoMethodError:
       undefined method `size' for nil:NilClass

Could you update the text in line with this, so that this error is also listed there? Thanks a lot!

@jaredbeck
Copy link
Contributor Author

Based on the discussion in #207, it looks like this should be listed in the changelog under "Known Issue" rather than "Breaking Change". I will edit this PR accordingly.

@jaredbeck
Copy link
Contributor Author

Could you update the text in line with this, so that this error is also listed there? Thanks a lot!

Updated, thanks. Hope it helps changelog-readers like myself :)

Made it more factually correct.
@perlun
Copy link
Contributor

perlun commented Nov 20, 2017

Thanks @jaredbeck! I did a small tweak before mergin, hope that was OK. Pathname is technically not an IO object, it just "walks like one"...

2.4.1 :004 > Pathname.new(Dir.pwd).is_a?(IO)
 => false

@perlun perlun merged commit 48dc4b3 into rack:master Nov 20, 2017
@jaredbeck jaredbeck deleted the patch-2 branch November 20, 2017 22:02
alex-damian-negru pushed a commit to alex-damian-negru/rack-test that referenced this pull request Apr 5, 2021
* Docs: Breaking change in 0.8.0 re: UploadedFile.new

[ci skip]

* Update History.md

* Update History.md

* Update History.md

Made it more factually correct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants