Add a comment encouraging developers to move js to bottom #5921

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@gaurish
Contributor

gaurish commented Apr 21, 2012

Relevant discussion here

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Apr 21, 2012

Contributor

Thanks for the patches but anyone that sees this is going to ask: why javascript_include_tag isn't in the bottom in the first place then?

Rails needs to decide if it is going to ship with it in the bottom or not. A comment is pretty much going to cause confusion and many people may end up doing the change without understanding the trade-offs, running in the issues we were trying to avoid in the first place (by not shipping with it in the bottom).

Finally, if people really think this change is fine, the comment should be made in ERb, so it doesn't appear on the rendered HTML.

Contributor

josevalim commented Apr 21, 2012

Thanks for the patches but anyone that sees this is going to ask: why javascript_include_tag isn't in the bottom in the first place then?

Rails needs to decide if it is going to ship with it in the bottom or not. A comment is pretty much going to cause confusion and many people may end up doing the change without understanding the trade-offs, running in the issues we were trying to avoid in the first place (by not shipping with it in the bottom).

Finally, if people really think this change is fine, the comment should be made in ERb, so it doesn't appear on the rendered HTML.

@gaurish

This comment has been minimized.

Show comment Hide comment
@gaurish

gaurish Apr 21, 2012

Contributor

@josevalim so what you suggest should be done?

Contributor

gaurish commented Apr 21, 2012

@josevalim so what you suggest should be done?

@kurko

This comment has been minimized.

Show comment Hide comment
@kurko

kurko Apr 21, 2012

@gaurish you could do just what @josevalim said and see if it gets accepted.

  • put the default javascript_include_tag in the bottom of the layout file
  • write the comment, if any, in ERb, not pure HTML, like he said?

kurko commented Apr 21, 2012

@gaurish you could do just what @josevalim said and see if it gets accepted.

  • put the default javascript_include_tag in the bottom of the layout file
  • write the comment, if any, in ERb, not pure HTML, like he said?
@drogus

This comment has been minimized.

Show comment Hide comment
@drogus

drogus Apr 21, 2012

Member

@josevalim @kurko see #5815 for the initial discussion

Member

drogus commented Apr 21, 2012

@josevalim @kurko see #5815 for the initial discussion

@gaurish

This comment has been minimized.

Show comment Hide comment
@gaurish

gaurish Jun 13, 2012

Contributor

Hey Guys ! Should I close this PR and instead update guides about this?

Contributor

gaurish commented Jun 13, 2012

Hey Guys ! Should I close this PR and instead update guides about this?

@drogus

This comment has been minimized.

Show comment Hide comment
@drogus

drogus Jun 26, 2012

Member

@gaurish yes, please. It seems that there is no consensus on what to do and I'm still not sure if we should add such comments to generated code.

Member

drogus commented Jun 26, 2012

@gaurish yes, please. It seems that there is no consensus on what to do and I'm still not sure if we should add such comments to generated code.

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