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

Rack::File -> Rack::Files #305

Closed
wants to merge 3 commits into from
Closed

Rack::File -> Rack::Files #305

wants to merge 3 commits into from

Conversation

postmodern
Copy link
Contributor

Renamed Rack::File to Rack::Files, since it can serve arbitrary files from a root directory. Once we can remove Rack::File, we can no longer worry about name conflicts between ::File and Rack::File.

@postmodern
Copy link
Contributor Author

To be clear, I left a lib/rack/file.rb file and Rack::File constant, which can be removed on a major version bump. Although, I'm unsure if or when a deprecation warning message should be added to the lib/rack/file.rb file, or if a deprecation notice in the ChangeLog would suffice?

@krainboltgreene
Copy link

This is an intelligent change, and makes literary and context sense.

@josevalim
Copy link
Contributor

Backwards incompatible. IIRC Rails relies on Rack::File (and potentially others tools besides Rails).

@krainboltgreene
Copy link

So mark it for a point release.

On Jan 12, 2012, at 1:34 PM, José Valimreply@reply.github.com wrote:

Backwards incompatible. IIRC Rails relies on Rack::File (and potentially others tools besides Rails).


Reply to this email directly or view it on GitHub:
#305 (comment)

@josevalim
Copy link
Contributor

It needs to be deprecated first.

@postmodern
Copy link
Contributor Author

@josevalim Like I said before, I left a Rack::File and rack/file.rb behind specifically for backwards compatibility. I'm leaving it up to the Rack team how they wish to deprecate the constant (I didn't get a clear answer as to how they normally handle deprecations).

@raggi
Copy link
Member

raggi commented Jan 23, 2012

Needs rebasing now. I'm not ignoring this, I'm just trying to decide when I want to pull the trigger. I think my answer is "for rails 4". The main point being, I don't really want to maintain too much heavy deviance and do continual cross-patching. I'll keep you posted ofc.

@postmodern
Copy link
Contributor Author

Rebased! Tests pass.

Should we bring @rkh into this discussion, as changes to Rack can also impact Sinatra? Also, until Rack::File is completely removed, all Rack code will have to continue using the awkward ::File style. There should be a grace period where Rack::File and Rack::Files co-exist.

@raggi
Copy link
Member

raggi commented Jan 25, 2012

The issues will be broad I believe. I'd expect this will also hit ramaze and maybe padrino separately.

Happy to add a deprecation notice in the next 1.4.x release. I'm not sure how long to give it, but I'd like to give it a while to avoid lots of merge issues. On that note, to reduce merge conflicts, I'd prefer to alias to Files first, and move it over when the deprecation is executed. This will save on maintenance time. (see multipart back ports :'( )

@raggi
Copy link
Member

raggi commented Jan 25, 2012

Thanks for rebasing!!!

@raggi
Copy link
Member

raggi commented Jan 22, 2013

Due to signature changes that have already occurred, I'm leaving this until 1.6 at the earliest.

@tenderlove
Copy link
Member

This seems fine to me. AFAIK, master is 1.6. Should we merge this? < @raggi

@spastorino spastorino modified the milestones: Rack 2.0, Rack 1.6 Nov 26, 2014
@ioquatix
Copy link
Member

@tenderlove I can rebase and merge this. I can add a deprecation notice and aim for removing in 3.0?

@ioquatix ioquatix modified the milestones: Rack 2.0, v3.0.0 Nov 14, 2019
@ioquatix ioquatix self-assigned this Nov 14, 2019
@tenderlove
Copy link
Member

@ioquatix sounds great! Go for it

@ioquatix
Copy link
Member

Okay rebasing today.

@ioquatix
Copy link
Member

Merged.

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.

None yet

7 participants