Refactor post and draft object creation #1779

Merged
merged 2 commits into from Dec 7, 2013

Projects

None yet

3 participants

@mattr-
Member
mattr- commented Dec 6, 2013

De-duplicate object creation between posts and drafts. Inject the name
dependency through a parameter

@mattr-
Member
mattr- commented Dec 6, 2013

/cc @pzula because she helped. 😃

@mattr-
Member
mattr- commented Dec 6, 2013

LOL!! didn't even think about that.

On Thu, Dec 5, 2013 at 11:28 PM, Parker Moore notifications@github.com
wrote:

Fails big time in 1.8.7

Reply to this email directly or view it on GitHub:
#1779 (comment)

@parkr
Member
parkr commented Dec 6, 2013

One of the main reasons I ❤️ continuous integration. If it had the green light, I definitely would have merged!

For reference, the error:

NoMethodError: undefined method `each_with_object' for #Array:0x7f2424e91688

@mattr-
Member
mattr- commented Dec 6, 2013

😭

@pzula
Contributor
pzula commented Dec 6, 2013

@mattr- Can we move back to using just plain each again and shoveling into an empty array?

@pzula
Contributor
pzula commented Dec 6, 2013

@mattr- Actually, I'm going to give good ol' map a try and rerun the tests, and see what TravisCI has to say about it

@pzula
Contributor
pzula commented Dec 6, 2013

@mattr- I've changed our enumerator and the tests all pass. However, I've never tried to update a PR before that was not my own... I have made the code changes in my fork here: pzula@440d3fd but am unsure as to how to add it to this pull request? Do I open a new request?

@parkr
Member
parkr commented Dec 6, 2013

@pzula Cherry-picked your commit into this branch. All set!

mattr- and others added some commits Dec 6, 2013
@mattr- @parkr mattr- Refactor post and draft object creation
De-duplicate object creation between posts and drafts. Inject the name
dependency through a parameter
e69d77b
@pzula @parkr pzula Modifying enumerator for 1.8.7 6a13f7d
@parkr
Member
parkr commented Dec 6, 2013

The failures I saw from the previous push were re: TOML stuff which I already fixed on master so I did a force push (heh).

Now watching this build closely...

@mattr- mattr- merged commit 362d28f into master Dec 7, 2013
@mattr- mattr- deleted the no-duplication branch Dec 7, 2013
@mattr- mattr- added a commit that referenced this pull request Dec 7, 2013
@mattr- mattr- Update history to reflect merge of #1779 8b0ea62
@parkr parkr commented on the diff Dec 9, 2013
lib/jekyll/site.rb
- # first pass processes, but does not yet render post content
- entries.each do |f|
- if Post.valid?(f)
- post = Post.new(self, self.source, dir, f)
-
- if post.published && (self.future || post.date <= self.time)
- aggregate_post_info(post)
- end
+ posts.each do |post|
+ if post.published && (self.future || post.date <= self.time)
@parkr
parkr Dec 9, 2013 Member

Is this supposed to be post.published?

@mattr-
mattr- Dec 9, 2013 Member

does published? do the same thing as published (without the question mark)? (this is rhetorical, I'll answer this myself)

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