Skip to content
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

make sure pages with published being false are not generated #1931

Merged
merged 1 commit into from
Jan 22, 2014

Conversation

liufengyun
Copy link
Contributor

In the documentation it says published can be used both in page and post. But it's only effective in post.

This commit makes published front matter also effective for pages. It also refactors the current implementation of published feature.

@parkr
Copy link
Member

parkr commented Jan 12, 2014

I'm cool with making the behaviour consistent between the two models. @mattr-, what do you think?

@ghost ghost assigned mattr- Jan 12, 2014
@liufengyun
Copy link
Contributor Author

@mattr- what do you think of this pull request?

@@ -21,6 +21,11 @@ def to_s
self.content || ''
end

# Helper methods to query boolean values specified in YAML frontmatter
%w(published).each do |key|
define_method("#{key}?") { !(self.data.has_key?(key) && self.data[key] == false) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have to implement this method this way? What prevents us from defining a normal method here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattr- I was thinking about later we may add similar methods to access boolean values in YAML front matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattr- I think you're right here, it's bad to do over-design with inelegant code. I've updated the commit, please have a look.

@parkr
Copy link
Member

parkr commented Jan 22, 2014

@mattr- When you get a minute, please review & merge if it looks good to you :)

@mattr-
Copy link
Member

mattr- commented Jan 22, 2014

chuck-norris

mattr- added a commit that referenced this pull request Jan 22, 2014
@mattr- mattr- merged commit 9885783 into jekyll:master Jan 22, 2014
mattr- added a commit that referenced this pull request Jan 22, 2014
@liufengyun
Copy link
Contributor Author

Thanks @mattr- 👍

@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants