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

Combined layout and theme features into one #1151

Merged
merged 2 commits into from Jun 8, 2013

Conversation

Projects
None yet
5 participants
@jpiasetz
Contributor

jpiasetz commented May 25, 2013

No description provided.

File.open(File.join('_layouts', layout + '.html'), 'w') do |f|
f.write(text)
Given /^I have an? (.*) (layout|theme) that contains "([^"]+)"$/ do |name, type, text|
folder = if (type == 'layout')

This comment has been minimized.

@parkr

parkr May 25, 2013

Member

You don't need the parentheses here :)

@parkr

This comment has been minimized.

Member

parkr commented May 25, 2013

Simplification always gets a 👍 from me! @mattr-?

@mattr-

This comment has been minimized.

Member

mattr- commented Jun 2, 2013

😐

I'd actually rather see the cucumber steps consolidated to only use the term 'layout' and remove any references to 'theme' altogether. I'd prefer to stick with the language of our domain, which is 'layout'.

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Jun 3, 2013

IIRC it's called theme intentionally, to test custom layout directories: https://github.com/mojombo/jekyll/blob/master/features/site_configuration.feature#L187. So you'd need another way to specify that it's in the _theme directories.

@jpiasetz

This comment has been minimized.

Contributor

jpiasetz commented Jun 3, 2013

@maul-esel so would it make more sense to change it to a feature called called custom layout folder?

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Jun 5, 2013

@jpiasetz: My intention was to point out why the term theme is essential to the feature and can not be replaced by layout, as @mattr- had suggested. I don't see any benefit in renaming it.

@parkr

This comment has been minimized.

Member

parkr commented Jun 6, 2013

@maul-esel I agree that it can't be named _layouts but I don't think we're tied to the term theme by any means.

@mattr- I'm 👍 on this now.

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Jun 6, 2013

@parkr: That's what I meant, sorry I was unclear.

mattr- added a commit that referenced this pull request Jun 8, 2013

Merge pull request #1151 from jpiasetz/refactors-steps
Combined layout and theme features into one

@mattr- mattr- merged commit a75b4a8 into jekyll:master Jun 8, 2013

1 check passed

default The Travis CI build passed
Details

mattr- added a commit that referenced this pull request Jun 8, 2013

@jpiasetz jpiasetz deleted the jpiasetz:refactors-steps branch Jun 9, 2013

parkr added a commit that referenced this pull request Jun 9, 2013

Merge branch 'master' of github.com:mojombo/jekyll
* 'master' of github.com:mojombo/jekyll:
  Update history to reflect merge of #1191
  Update history to reflect merge of #1151
  Moving Quick-start guide to own section
  Remove extra parentheses and swapped regex back
  Combined two features into one

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