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

Exclude Gemfile by default #5860

Merged
merged 5 commits into from Feb 11, 2017

Conversation

Projects
None yet
5 participants
@ashmaroli
Member

ashmaroli commented Feb 4, 2017

Exclude Gemfile and Gemfile.lock by default. I can't think of any situation where they may be needed to be processed.

Additionally, comment out existing exclude: array in the generated _config.yml and explain what to do with it

Addresses #5859

--
/cc @jekyll/ecosystem

@parkr

Love it! Left a comment about wording but otherwise 👍

Show outdated Hide outdated lib/site_template/_config.yml
# Exclude from processing.
# The items below are excluded by default, to customize the list, uncomment the
# lines below and simply add to the list. Items removed will **not** be excluded
# from processing.

This comment has been minimized.

@parkr

parkr Feb 4, 2017

Member

This comment is a little confusing to me. I think what you're driving at is that the defaults go away if you create a custom exclude here. Let's try to be really really precise here.

@parkr

parkr Feb 4, 2017

Member

This comment is a little confusing to me. I think what you're driving at is that the defaults go away if you create a custom exclude here. Let's try to be really really precise here.

This comment has been minimized.

@ashmaroli

ashmaroli Feb 5, 2017

Member

Yes, that's indeed the intention.

@ashmaroli

ashmaroli Feb 5, 2017

Member

Yes, that's indeed the intention.

@DirtyF

DirtyF approved these changes Feb 5, 2017 edited

👍 Perfect

@parkr

parkr approved these changes Feb 11, 2017

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 11, 2017

Member

@jekyllbot: merge +bug

Member

parkr commented Feb 11, 2017

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit afe6e36 into jekyll:master Feb 11, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot added bug fix labels Feb 11, 2017

jekyllbot added a commit that referenced this pull request Feb 11, 2017

@pathawks pathawks referenced this pull request Feb 13, 2017

Closed

Invalid date error resolution #5359

6 of 17 tasks complete
# Exclude from processing.
# The following items will not be processed, by default. Create a custom list
# to override the default setting.
# exclude:

This comment has been minimized.

@pathawks

pathawks Feb 13, 2017

Member

I wonder if there's anyway this list could be automatically updated with the default exclude list?

@pathawks

pathawks Feb 13, 2017

Member

I wonder if there's anyway this list could be automatically updated with the default exclude list?

This comment has been minimized.

@ashmaroli

ashmaroli Feb 13, 2017

Member

we'll probably have to modify the deep_merge_hashes utility method to append! when hash[key].is_a? Array ...

@ashmaroli

ashmaroli Feb 13, 2017

Member

we'll probably have to modify the deep_merge_hashes utility method to append! when hash[key].is_a? Array ...

This comment has been minimized.

@ashmaroli

ashmaroli Feb 13, 2017

Member

.. I never added that patch to jekyll-data plugin because then one would never be able to take an item off the list if they needed to..

@ashmaroli

ashmaroli Feb 13, 2017

Member

.. I never added that patch to jekyll-data plugin because then one would never be able to take an item off the list if they needed to..

This comment has been minimized.

@pathawks

pathawks Feb 13, 2017

Member

But we would just want it to be comments rather than an actual list.

I just worry about adding files to the exclude list and forgetting to document the change here. We'll just have to be cautious, I guess

@pathawks

pathawks Feb 13, 2017

Member

But we would just want it to be comments rather than an actual list.

I just worry about adding files to the exclude list and forgetting to document the change here. We'll just have to be cautious, I guess

This comment has been minimized.

@ashmaroli

ashmaroli Feb 14, 2017

Member

@pathawks, it just struck me.. we could have the list automatically updated if _config.yml was generated from an ERB template _config.yml.erb :

# exclude:
<% for item in Jekyll::Configuration::DEFAULTS["exclude"] %>
#   - <%= item %>
<% end %>

concern: Matt's comment from many months ago regarding the use of ERB templates..

Your thoughts ?

@ashmaroli

ashmaroli Feb 14, 2017

Member

@pathawks, it just struck me.. we could have the list automatically updated if _config.yml was generated from an ERB template _config.yml.erb :

# exclude:
<% for item in Jekyll::Configuration::DEFAULTS["exclude"] %>
#   - <%= item %>
<% end %>

concern: Matt's comment from many months ago regarding the use of ERB templates..

Your thoughts ?

@ashmaroli ashmaroli deleted the ashmaroli:exclude-patch branch Oct 29, 2017

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