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

Update documentation for Rails::Application#env_config #7070

Merged
merged 1 commit into from Aug 1, 2012

Conversation

jmazzi
Copy link
Contributor

@jmazzi jmazzi commented Jul 16, 2012

The documentation for Rails::Application#env_config was incorrect and missing some information. This pull request fixes it.

@rafaelfranca
Copy link
Member

I think we can remove these information as we did at master.

https://github.com/rails/rails/blob/master/railties/lib/rails/application.rb#L103-104

If they want to know what are configurations they can see with the "view source" link at the API page.

WDYT?

cc/ @fxn @vijaydev

@jmazzi
Copy link
Contributor Author

jmazzi commented Jul 16, 2012

That sounds reasonable to me. Let me know if that's what you want me to do.

@fxn
Copy link
Member

fxn commented Jul 21, 2012

If these keys are public interface, I think they should be documented normally. People should not need to inspect the source code IMO. Why were they removed from master?

@rafaelfranca
Copy link
Member

I don't know. If we are going to document it, we need to do in master too.

@fxn
Copy link
Member

fxn commented Jul 21, 2012

Great, @jmazzi would you volunteer a patch for master?

@vijaydev
Copy link
Member

I was the one who removed them in master here. Back then, the doc didn't match the actual list of configs. My thinking at the time was that it's hard to keep the list in sync, unless people do it diligently.

But yeah, I agree with @fxn that we should document it properly.

@jmazzi
Copy link
Contributor Author

jmazzi commented Jul 21, 2012

Yep, will do.

On Jul 21, 2012, at 3:32 AM, Xavier Noria
reply@reply.github.com
wrote:

Great, @jmazzi would you volunteer a patch for master?


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

@fxn
Copy link
Member

fxn commented Jul 21, 2012

@vijaydev perfect!

@jmazzi
Copy link
Contributor Author

jmazzi commented Aug 1, 2012

Master has been updated. This can also be pulled unless further change is needed.

jmazzi added a commit to jmazzi/rails that referenced this pull request Aug 1, 2012
rafaelfranca added a commit that referenced this pull request Aug 1, 2012
Update documentation for Rails::Application#env_config
@rafaelfranca rafaelfranca merged commit 3e01a2a into rails:3-2-stable Aug 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants