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

Demo minima on GitHub Pages #76

Merged
merged 4 commits into from
Apr 7, 2017
Merged

Demo minima on GitHub Pages #76

merged 4 commits into from
Apr 7, 2017

Conversation

DirtyF
Copy link
Member

@DirtyF DirtyF commented Nov 8, 2016

fix #67

/cc @jekyll/minima

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

LGTM! @benbalter, seem OK to you?

@pathawks
Copy link
Member

pathawks commented Nov 9, 2016

This would not allow the example site to showcase the current version of Minima, but would rely on the version included in GitHub Pages, is that correct?

@benbalter
Copy link
Contributor

This would not allow the example site to showcase the current version of Minima, but would rely on the version included in GitHub Pages, is that correct?

Correct. I'm fine with it essentially showing the released version, unless there's any objection.

@ashmaroli
Copy link
Member

I'm 👎 on this primarily because:

  • IMHO, a Jekyll theme-repo root should only contain directories relevant to the theme, plus a directory named either _example_ or _demo_ which will contain files and directories generated by the latest version of Jekyll.
  • The separation of theme-gem files and source-files will help us showcase the actual working relationships between a theme-gem-dir and source-dir:
    • _layouts, _includes, _sass & assets may not be present inside the source_dir to be read by Jekyll
    • The files inside directories above, if present inside same directories at source_dir, will override theme templates. (currently not demo-ed, though)

@benbalter
Copy link
Contributor

@DirtyF Thanks for implementing this and sorry it took so long to review.

In order to facilitate demos, local development, and legacy theme workflows (fork and edit), Jekyll themes were intended to be just Jekyll sites that had a Gemspec added. I really like the simplicity this PR allows for and how much it simplifies the local development experience. Anyone who knows how to edit a Jekyll site can now contribute to minima. Thank you!

@jekyllbot: merge +dev.

@jekyllbot jekyllbot merged commit a4726e6 into jekyll:master Apr 7, 2017
jekyllbot added a commit that referenced this pull request Apr 7, 2017
@ashmaroli
Copy link
Member

@benbalter Please add the preview link on the repo's index page

@benbalter benbalter mentioned this pull request Apr 7, 2017
@alxn
Copy link
Contributor

alxn commented Apr 7, 2017

@ashmaroli I guess like the rest of the themes:

XXXX is a Jekyll theme for GitHub Pages. You can preview the theme to see what it looks like, or even use it today.

@DirtyF DirtyF deleted the demo branch April 7, 2017 17:33
@ashmaroli
Copy link
Member

@alxn I agree with your point. But I also liked how earlier I could make the distinction between a Jekyll Site and the Minima gem contents while developing...

@ashmaroli
Copy link
Member

To add to my point above, say, I add a _data folder to the repo. Earlier I'd add it to the example/ and my users would know its not included in the gem.. Now they'll have to inspect the gemspec and interpret the regex to do the same..

domingohui pushed a commit to domingohui/minima that referenced this pull request Apr 14, 2017
domingohui pushed a commit to domingohui/minima that referenced this pull request Apr 14, 2017
@jekyll jekyll locked and limited conversation to collaborators Apr 29, 2019
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.

Demo
7 participants