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

Set Timezone in _config.yml #957

Merged
merged 14 commits into from Apr 15, 2013

Conversation

Projects
None yet
4 participants
@parkr
Member

parkr commented Apr 13, 2013

This PR allows the user to set the timezone (values allowed in TZ) in the configuration file (e.g. _config.yml), which will be observed when the site is built.

Ref: #698

  • Set timezone for process from config
  • Docs for timezone configuration and how it works
  • Cucumber tests for timezone switching
@mattr-

This comment has been minimized.

Member

mattr- commented Apr 14, 2013

ok so far.

@mojombo

This comment has been minimized.

Contributor

mojombo commented Apr 14, 2013

Definitely need docs on this option. As a user, I need to know what format you want for TZ.

@parkr

This comment has been minimized.

Member

parkr commented Apr 14, 2013

Added some docs under the "Global Configuration" section on the "Configuration" page. The IANA Time Zone Database is a link to its Wikipedia page, which has a link to the list of all time zone names.

Screen Shot 2013-04-14 at 8 23 41 PM

@mojombo

This comment has been minimized.

Contributor

mojombo commented Apr 14, 2013

@parkr Perfect!

@parkr

View changes

features/step_definitions/jekyll_steps.rb Outdated
post['date'] = parsed_date.to_s
parsed_date.strftime('%Y-%m-%d')
else
parsed_date = Date.strptime(post['date'], '%m/%d/%Y') # WHY WOULD YOU EVER DO THIS

This comment has been minimized.

@parkr

parkr Apr 14, 2013

Member

WHO DECIDED WE WERE GOING TO USE MM/DD/YYYY I JUST SPEND 2 HOURS DEBUGGING THIS CRAPOLA I HATE CUCUMBER %^@$(!&^&@$&!(@!@%^#$%^!@%^@!#!$#@#!@#!$!!#((&!@)$:">{>":"

@parkr

This comment has been minimized.

Member

parkr commented Apr 14, 2013

@mattr- @mojombo Doneeeeeezo.

@@ -116,6 +116,46 @@ Feature: Site configuration
And I should see "Post Layout: <p>content for entry1.</p>" in "_site/2007/12/31/entry1.html"
And I should see "Post Layout: <p>content for entry2.</p>" in "_site/2020/01/31/entry2.html"
Scenario: Generate proper dates with explicitly set timezone (which is the same)

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

I would change '(which is the same)' to be '(same as current time zone)'. The way is currently written made me go, "huh?"

@@ -1,4 +1,7 @@
Before do
if File.directory?(TEST_DIR)

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

Why does it matter if TEST_DIR is a directory here?

This comment has been minimized.

@parkr

parkr Apr 15, 2013

Member

If it exists, we need to delete it. I guess it can just be File.exists?

@@ -61,7 +64,15 @@
if "draft" == status
path = File.join(before || '.', '_drafts', after || '.', "#{title}.#{ext}")
else
date = Date.strptime(post['date'], '%m/%d/%Y').strftime('%Y-%m-%d')
date = if post['date'].split.size > 1

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

Can we encapsulate post['date'].split.size > 1 to be has_time_component? or something similar?

# definition
def has_time_component?(date_string)
  date_string.split.size > 1
end

#usage
date = if has_time_component?(post['date'])

Looks cleaner from my point of view.

parsed_date.strftime('%Y-%m-%d')
else
parsed_date = Date.strptime(post['date'], '%m/%d/%Y') # WHY WOULD YOU EVER DO THIS
post['date'] = parsed_date.to_s

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

Lines 73 and 74 duplicate lines 69 and 70 above. Can we get rid of that somehow?

# config - the Jekyll::Configuration generated by Jekyll.configuration
#
# Returns nothing
def self.set_timezone(config)

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

why pass in a config object when we can just pass in the actual timezone value? (just curious)

This comment has been minimized.

@parkr

parkr Apr 15, 2013

Member

this way i can check for the value in the method. otherwise, i have to clutter up the self.configuration method with an if statement that i'd rather avoid

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

I must be missing something. Can't you check the value in the method when you pass in just the value?

Never mind. Went back and read this in the context of the full file. Totally ok with this. 😄

<p class='name'><strong>Time Zone</strong></p>
<p class="description">
Set the time zone for site generation. This sets the <code>TZ</code>
shell environment variable, which Ruby uses to handle time and date

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

It feels like the word 'shell' is redundant here, but that could just be me.

@@ -1,4 +1,7 @@
Before do
if File.exists?(TEST_DIR)

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

I don't even think we need a check for existence here, which is what I actually meant by my prior comment. Sorry, I should have been more clear before.

This comment has been minimized.

@parkr

parkr Apr 15, 2013

Member

Ah. Then rm_rf will not throw an error if we try to delete something that is not there?

This comment has been minimized.

@mattr-

mattr- Apr 15, 2013

Member

Nope, that's why you use rm -rf cause ain't nobody got time for that. 😉

This comment has been minimized.

@parkr

parkr Apr 15, 2013

Member

It ignores StandardError when it's set to :force, so you're right, it should be fine.

@parkr

This comment has been minimized.

Member

parkr commented Apr 15, 2013

@mattr- Good to go, then? :)

@mattr-

This comment has been minimized.

Member

mattr- commented Apr 15, 2013

👍 :shipit:

parkr added a commit that referenced this pull request Apr 15, 2013

@parkr parkr merged commit afa3ef9 into master Apr 15, 2013

@parkr parkr deleted the timezonify branch Apr 15, 2013

parkr added a commit that referenced this pull request Apr 15, 2013

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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