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

Avoid conflict with existing mime gem #100

Merged
merged 1 commit into from
May 24, 2015

Conversation

blanquer
Copy link
Contributor

  • Moved lib/mime.rb away from top-level (otherwise the loading paths would use the file instead of the existing gem’s).
  • Still apply the compatibility patch on the MIME module.

@halostatue
Copy link
Member

I’m neutral to negative about this change. I need to think about it.

@blanquer
Copy link
Contributor Author

is there any downside to doing this? Are you guys worrying that somebody uses require 'mime' but somehow assuming that it pulls mime-types code?

It really makes it not possible to use the mime gem, cause messing with the load paths, conditionally is a really bad approach

Let me elaborate a little more about the use case so you see how difficult it is to deal with it. I am a maintainer of the Praxis framework. It is a Web framework that uses the mime gem for multipart-related parsing and generation. Now, we've gotten issues open from people building Praxis apps that require gems like rest-client (which pulls in mime-types).

So...from a framework perspective it's all good, no conflict. But now any application using it is stuck with a big problem because pulling in mime-types will likely result in a load path that puts its "lib" directory before the mime gem "lib" directory...which means that anywhere where there's a require 'mime' will end up just requiring your lib/mime.rb file (instead of the expected file from the mime gem).

I'm not really sure there's even a good workaround from the app's perspective to resolve that other than doing something very dirty in re-ordering the loadpath.

Anyway, this is the issue (and specific use-case) at hand, so please think about it...it is really an important compatibility issue.

@halostatue
Copy link
Member

I appreciate the deeper explanation. There isn’t a good workaround, as you noted. Where I think that I’m not comfortable is the name you chose (mime/types/compat.rb) because it isn’t what’s being done here. Change it to mime/deprecations.rb and I think I can take this in even though I don’t like the reason for it.

@blanquer
Copy link
Contributor Author

I appreciate that. I've pushed the name change.

I didn't commit the .gemspec since I'm assuming you manage that on your own in your releases.
Thank you!

@halostatue
Copy link
Member

Can I get you to squash the two commits here into a single commit? I’ll be happy to merge it at that point; I’m planning on releasing 2.6 in the next couple of days (I have some documentation to finish).

* Moved lib/mime.rb away from top-level (otherwise the loading paths would use the file instead of the existing gem’s).
* Still apply the compatibility patch on the `MIME` module.
@blanquer blanquer force-pushed the avoid_mime_gem_conflict branch from 29d0bfe to 2a1f09c Compare May 24, 2015 02:47
@blanquer
Copy link
Contributor Author

You bet. Here you have it.

halostatue added a commit that referenced this pull request May 24, 2015
Avoid conflict with existing `mime` gem
@halostatue halostatue merged commit 492f769 into mime-types:master May 24, 2015
@halostatue
Copy link
Member

Thanks, @blanquer.

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Jun 8, 2015
== 2.6.1 / 2015-05-25

* Bugs:
  * Make columnar store handle all supported extensions, not just the first.
  * Avoid circular require when using the columnar store.

== 2.6 / 2015-05-25

* New Feature:
  * Columnar data storage for the MIME::Types registry, contributed by Jeremy
    Evans (@jeremyevans). Reduces default memory use substantially (the mail
    gem drops from 19 Mib to about 3 Mib). Resolves
    {#96}[mime-types/ruby-mime-types#96],
    {#94}[mime-types/ruby-mime-types#94],
    {#83}[mime-types/ruby-mime-types#83]. Partially
    addresses {#64}[mime-types/ruby-mime-types#64]
    and {#62}[mime-types/ruby-mime-types#62].
* Development:
  * Removed caching of deprecation messages in preparation for mime-types 3.0.
    Now, deprecated methods will always warn their deprecation instead of only
    warning once.
  * Added a logger for deprecation messages.
  * Renamed <tt>lib/mime.rb</tt> to <tt>lib/mime/deprecations.rb</tt> to not
    conflict with the {mime}[https://rubygems.org/gems/mime] gem on behalf of
    the maintainers of the {Praxis Framework}[http://praxis-framework.io/].
    Provided by Josep M. Blanquer (@blanquer),
    {#100}[mime-types/ruby-mime-types#100].
  * Added the columnar data conversion tool, also provided by Jeremy Evans.
* Documentation:
  * Improved documentation and ensured that all deprecated methods are marked
    as such in the documentation.
* Development:
  * Added more Ruby variants to Travis CI.
  * Silenced deprecation messages for internal tools. Noisy deprecations are
    noisy, but that's the point.

== 2.5 / 2015-04-25

* Bugs:
  * David Genord (@albus522) fixed a bug in loading MIME::types cache where a
    container loaded from cache did not have the expected +default_proc+,
    {#86}[mime-types/ruby-mime-types#86].
  * Richard Schneeman (@schneems) provided a patch that substantially reduces
    unnecessary allocations.
* Documentation:
  * Tibor Szolár (@flexik) fixed a typo in the README,
    {#82}[mime-types/ruby-mime-types#82]
  * Fixed {#80}[mime-types/ruby-mime-types#80],
    clarifying the relationship of MIME::Type#content_type and
    MIME::Type#simplified, with Ken Ip (@kenips).
* Development:
  * Juanito Fatas (@JuanitoFatas) enabled container mode on Travis CI,
    {#87}[mime-types/ruby-mime-types#87].
* Moved development to a mime-types organization under
  {mime-types/ruby-mime-types}[https://github.com/mime-types/ruby-mime-types].
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants