Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Edge guides don't properly link to the current GitHub tree #8221

Closed
wants to merge 1 commit into from

5 participants

@agrobbin

WIthout the full commit hash, GitHub responds with a 404 when clicking the edge guides "master branch" link. I would have committed this directly to lifo/docrails, but it does include code-related changes in generator.rb, so I thought it was warranted to put it here first.

@rafaelfranca

cc @fxn

@steveklabnik
Collaborator

:+1:

@vijaydev vijaydev commented on the diff
guides/rails_guides/generator.rb
@@ -85,6 +85,7 @@ def set_flags_from_environment
@all = ENV['ALL'] == '1'
@kindle = ENV['KINDLE'] == '1'
@version = ENV['RAILS_VERSION'] || `git rev-parse --short HEAD`.chomp
+ @full_version = ENV['RAILS_VERSION'] || `git rev-parse HEAD`.chomp
@vijaydev Collaborator

if the short SHA is sent in the ENV variable, this is still a problem right? I 'd suggest to remove the ENV check for the full version.

Is there any time that ENV['rails_version'] would be passed when it wouldn't be a branch or tag name? Or rather, when it would be a short commit hash?

@vijaydev Collaborator

I'm not sure how it is set up on the docs server. @fxn can tell.

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

Oh, interesting. I am sure that worked in the past.

We have two use cases here.

In the stable docs, the version is the git tag, eg "v3.2.9". The shell script that detects there is a new stable version passes the tag name to the stable docs generator via RAILS_VERSION, and it is used both in the title (within parens), and in the link of the preamble.

In the edge docs, I believe we could also pass the SHA1 in RAILS_VERSION and remove the backticks. To make it uniform. I would update the scripts in the server.

Then, if @edge is true a simple @version[0, 7] within parens would suffice for the title.

@agrobbin

How would you recommend I proceed with this pull request?

@agrobbin agrobbin closed this
@agrobbin agrobbin reopened this
@fxn fxn closed this pull request from a commit
@fxn fxn let @version be always externally set in guides generation [fixes #8221]
Shelling out was there for authors convenience, but we are
rather going to have the tag or SHA1 always in RAILS_VERSION
and if the environment variable is blank, then just use
"local" as a reminder that you are just working locally.

The docs server has been updated to set the long SHA1 in
RAILS_VERSION when generating edge guides.
0adcf6d
@fxn fxn closed this in 0adcf6d
@fxn
Owner
fxn commented

Hi, finally I have done this myself because it needed synchronization with generation scripts in the docs server. Thanks for noticing this bug.

@agrobbin

No problem, thanks for fixing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 14, 2012
This page is out of date. Refresh to see the latest.
View
1  guides/rails_guides/generator.rb
@@ -85,6 +85,7 @@ def set_flags_from_environment
@all = ENV['ALL'] == '1'
@kindle = ENV['KINDLE'] == '1'
@version = ENV['RAILS_VERSION'] || `git rev-parse --short HEAD`.chomp
+ @full_version = ENV['RAILS_VERSION'] || `git rev-parse HEAD`.chomp
@vijaydev Collaborator

if the short SHA is sent in the ENV variable, this is still a problem right? I 'd suggest to remove the ENV check for the full version.

Is there any time that ENV['rails_version'] would be passed when it wouldn't be a branch or tag name? Or rather, when it would be a short commit hash?

@vijaydev Collaborator

I'm not sure how it is set up on the docs server. @fxn can tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lang = ENV['GUIDES_LANGUAGE']
end
View
2  guides/source/_welcome.html.erb
@@ -10,7 +10,7 @@
</p>
<% else %>
<p>
- These are the new guides for Rails 3.2 based on <a href="https://github.com/rails/rails/tree/<%= @version %>"><%= @version %></a>.
+ These are the new guides for Rails 3.2 based on <a href="https://github.com/rails/rails/tree/<%= @full_version %>"><%= @version %></a>.
These guides are designed to make you immediately productive with Rails, and to help you understand how all of the pieces fit together.
</p>
<% end %>
Something went wrong with that request. Please try again.