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

Move the reading of layouts into its own class. #2020

Merged
merged 7 commits into from Feb 8, 2014

Conversation

Projects
None yet
4 participants
@mattr-
Member

mattr- commented Feb 7, 2014

Refactor the reading of layouts into its own class because it's not Site's
responsibility. This will also give us more flexibility when we start making
more changes to the rendering process

mattr- added some commits Nov 6, 2013

Sort methods from most important to least important
This idea is based on the concept of a newspaper. The most important
things such as the headlines and the major details of the story at the
top. This translates to code in that the public API and the more
important private methods are at the top of the file. The more detailed
information (or methods, in the code) are further down, so that if
you've gotten all you need out of the code up to a certain point, you
don't need to keep reading anymore.
def layout_entries
entries = []
with(layout_directory) do

This comment has been minimized.

@parkr

parkr Feb 7, 2014

Member

Would be so cool if you could use in here.

This comment has been minimized.

@mattr-

mattr- Feb 7, 2014

Member

like in(layout_directory)?

This comment has been minimized.

@mattr-

mattr- Feb 7, 2014

Member

and is that already a thing we have somewhere else?

This comment has been minimized.

@parkr

parkr Feb 7, 2014

Member

Yeah! 💃

This comment has been minimized.

@mattr-

mattr- Feb 7, 2014

Member

in is a keyword. so I can't write in(layout_directory). We'll write within(layout_directory) instead.

This comment has been minimized.

@parkr

parkr Feb 7, 2014

Member

Haha good compromise 😃

@parkr

This comment has been minimized.

Member

parkr commented Feb 7, 2014

We'll wait for Travis's verdict, but I'm super 👍 on this. Thanks 😃

@parkr parkr self-assigned this Feb 7, 2014

@parkr parkr added this to the 2.0 milestone Feb 7, 2014

@parkr parkr added the Refactor label Feb 7, 2014

@parkr

This comment has been minimized.

Member

parkr commented on 833b400 Feb 7, 2014

❤️

parkr added a commit that referenced this pull request Feb 8, 2014

@parkr parkr merged commit 8f3e3e0 into master Feb 8, 2014

1 check passed

default The Travis CI build passed
Details

@parkr parkr deleted the refactor-layout-reading branch Feb 8, 2014

parkr added a commit that referenced this pull request Feb 8, 2014

@cup

This comment has been minimized.

cup commented on f581940 Feb 9, 2014

While authored 3 months ago this was actually committed 3 days ago

$ git show -s --format=%ci f581940
2014-02-06 22:22:06 -0600

Performing a git bisect revealed this commit broke my jekyll

$ git checkout f581940

$ gem build jekyll.gemspec

$ gem install jekyll-1.4.3.gem

$ jekyll
/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:53:in `require':
                       cannot load such file -- jekyll/layout_reader (LoadError)
    from /lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:53:in `require'
    from /lib/ruby/gems/2.0.0/gems/jekyll-1.4.3/lib/jekyll.rb:51:
                                                             in `<top (required)>'
    from /lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:53:in `require'
    from /lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:53:in `require'
    from /lib/ruby/gems/2.0.0/gems/jekyll-1.4.3/bin/jekyll:6:in `<top (required)>'
    from /bin/jekyll:23:in `load'
    from /bin/jekyll:23:in `<main>'

This comment has been minimized.

Member

parkr replied Feb 9, 2014

I only just merged it: #2020. Has it broken master?

This comment has been minimized.

cup replied Feb 9, 2014

@parkr yes.

This comment has been minimized.

Member

parkr replied Feb 9, 2014

Fixed in f607aef

This comment has been minimized.

cup replied Feb 9, 2014

Well that was quick! Tested and confirmed fixed. Thanks!

parkr added a commit that referenced this pull request Feb 9, 2014

@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.