Skip to content

Consistency fix to the Getting Started with Rails guide #14417

Closed
wants to merge 1 commit into from

5 participants

@nixward
nixward commented Mar 18, 2014

This corrects the code snippet taken from the app/views/posts/new.html.erb
file on line 704. Earlier in the guide on lines 395-398 the guide user is
instructed to change the form_for line in the new.html.erb file to use the
posts_path helper. This fix updates the code snippet to be consistent with that
earlier change.

@nixward nixward Consistency fix to the Getting Started with Rails guide
This corrects the code snippet taken from the app/views/posts/new.html.erb
file.  Earlier in the guide on lines 395-398 the user is instructed to change the
form_for line in the new.html.erb file to use the posts_path helper.  This fix 
updates the code snippet to be consistent with that earlier change.
c877934
@fxn
Ruby on Rails member
fxn commented Mar 18, 2014

This seems backwards to me, it does not make sense to do first the view, with no model, and because of that do weird things like passing an old-fashioned symbol to form_for instead of a @post ivar that does the right thing out of the box.

Beginners should be presented idiomatic smooth paths, not rare for options like :url.

@fxn
Ruby on Rails member
fxn commented Mar 18, 2014

@nixw I'd rather say the url option is no longer needed because now Rails has information to compute the path.

@nixward
nixward commented Mar 19, 2014

Hi @fxn, maybe describing the issue as a consistency fix was wrong of me. The issue I see is the new.html.erb file is updated at various points in the guide but the snippet from the file I have corrected had reverted to a previous state without any reason, i.e. if a user is following the guide and follows the instruction at lines 395-398 the file state at line 704 of the guide should be:

<%= form_for :post, url: posts_path do |f| %>
  <p>
    <%= f.label :title %><br>
    <%= f.text_field :title %>
  </p>

  <p>
    <%= f.label :text %><br>
    <%= f.text_area :text %>
  </p>

  <p>
    <%= f.submit %>
  </p>
<% end %>

but the snippet is implying it to be:

<%= form_for :post do |f| %>
  <p>
    <%= f.label :title %><br>
    <%= f.text_field :title %>
  </p>

  <p>
    <%= f.label :text %><br>
    <%= f.text_area :text %>
  </p>

  <p>
    <%= f.submit %>
  </p>
<% end %>

So the issue I fixed was to stop the shown state of new.html.erb to mysteriously revert to a previous state. I hope that better explains the fix.

Also this is only a documentation change and I forgot to include [ci skip] into the commit message. Should I fix the commit message and make another pull request? Thanks.

@senny senny added the docs label Mar 20, 2014
@bambery
bambery commented Apr 4, 2014

@nixw is correct - there is a reference made to app/views/posts/new.html.erb without the url option on line 704, but the same code snippet in the same file is referred to with the url option both before (398, where the option was added) and after (805) this snippet, so line 704 is inconsistent.

Tangentially related, when the url option is eventually dropped off when moving the form into a partial (line 990), it changes from
<%= form_for :post, url: posts_path do |f| %>
to
<%= form_for @post do |f| %>
and the guide promises
How form_for can figure out the right action and method
attributes when building the form will be explained in just a moment.

(lines 1018-1020) but it never does. I'd suggest a fix, but I don't know the answer. If it's still there in a few days, I'll research it and submit a new CL.

@vijaydev
Ruby on Rails member
vijaydev commented May 2, 2014

The Getting Started guide has undergone quite a few changes of late and the version in stable (and edge) guides is not even using the posts example anymore. That said, I see the usage of url in the latest guides too - warrants a relook.

@nixw @bambery Please have a look at the latest guides and discuss in the rails docs mailing list if you need any clarifications. Changes to improve the guide are obviously ok as pull requests.

I'm closing this one, since this targets 4-0-stable and we wouldn't want to maintain multiple guides.

@vijaydev vijaydev closed this May 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.