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

Display latest version in header #6676

Merged
merged 2 commits into from Jan 6, 2018

Conversation

Projects
None yet
4 participants
@jekyllbot
Contributor

jekyllbot commented Jan 6, 2018

PR automatically created for @DirtyF.

Display latest version in header

@DirtyF DirtyF requested review from ashmaroli and Crunch09 Jan 6, 2018

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

I don't like to have to update the config file manually at each release, any way to add this to our rake release task?

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

jekyll/rake/site.rake

Lines 87 to 90 in 5ef2deb

desc "Write the site latest_version.txt file"
task :version_file do
File.open("#{docs_folder}/latest_version.txt", "wb") { |f| f.puts(version) } unless version =~ %r!(beta|rc|alpha)!i
end

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

@ashmaroli yeah I was thinking of something similar: a task that parses docs/_config.yml and replace the version value. 😄

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

yes, that's the way to do it.. I just pointed you to where to add the code..
Do you want me to push to this branch or just paste the code here..?

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

@ashmaroli as you like (I was going to try to learn how to parse a yml file in Ruby, but I 'm sure you'll be quicker at this). Thanks for your help.

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

This is the updated block for above snippet.. I'll leave it to you to update the task name(s) and description and test locally .. 😉 😁

  desc "Write the site latest_version.txt file"
  task :version_file do
    return if version =~ %r!(beta|rc|alpha)!i
    config_file = File.join(docs_folder, "_config.yml")
    contents = File.read(config_file)
    File.write(config_file, contents.sub(/(?:version\s*:\s+)(.+)/, "version: #{version}"))
  end
@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

@ashmaroli thanks, works like a charm 👍

desc "Write the site latest_version.txt file"
task :version_file do
File.open("#{docs_folder}/latest_version.txt", "wb") { |f| f.puts(version) } unless version =~ %r!(beta|rc|alpha)!i
desc "Write the site latest version"

This comment has been minimized.

@ashmaroli

ashmaroli Jan 6, 2018

Member

How about..?

- "Write the site latest version"
+ "Write the latest Jekyll version used to generate the site"

This comment has been minimized.

@DirtyF

DirtyF Jan 6, 2018

Member

I kept the beginning of your suggestion to keep it short

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

okay.. I'll let the others review the changes..
If you (and the others) feel like taking the task to another level, the following patch should insert the version data if its ever removed..

  desc "Write the latest Jekyll version"
  task :latest_version do
    return if version =~ %r!(beta|rc|alpha)!i
    config_file   = File.join(docs_folder, "_config.yml")
    contents      = File.read(config_file)
    version_regex = /(?:version\s*:\s+)(.+)/

    if contents =~ version_regex
      File.write(config_file, contents.sub(version_regex, "version: #{version}"))
    else
      File.open(config_file, "a+") do |file|
        file.puts <<-TXT

# latest docs version displayed in the header
# inserted and updated automatically by the following rake task(s)
#   bundle exec rake site:preview
#   bundle exec rake site:latest_version
version: #{version}"
TXT
      end
    end
  end
@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

@jekyllbot: merge +site

@jekyllbot jekyllbot merged commit 413de6a into master Jan 6, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the pull/site-version branch Jan 6, 2018

@Crunch09

This comment has been minimized.

Member

Crunch09 commented Jan 6, 2018

@DirtyF @ashmaroli ❤️ thanks for fixing this so quickly!
A nicer alternative might be:

require 'safe_yaml/load'
config = SafeYAML.load_file(config_file)
config["version"] = version
File.write(config_file, YAML.dump(config))

so we don't have to use a regex, but just a suggestion as it is already merged and works fine!

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

@Crunch09 👍 that's exactly what I was thinking about, I like it DRY. I even thought we could use a Jekyll class to read the config file.

Feel free to open a PR and I'll merge it 😄

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

A nicer alternative might be:

Wow @Crunch09, That is sooo much better..!! ( Why didn't I think of that!! 😄 )

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