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

Update version in `config.yml` via YAML load / dump #6677

Merged
merged 2 commits into from Jan 6, 2018

Conversation

Projects
None yet
4 participants
@Crunch09
Member

Crunch09 commented Jan 6, 2018

As discussed in #6676 , I've already ran the command once, so we can see what changes in the config.yml file when running this command (as it normalizes the YAML output)

@Crunch09 Crunch09 requested review from DirtyF and ashmaroli Jan 6, 2018

@DirtyF

DirtyF approved these changes Jan 6, 2018

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

@Crunch09 nice extra feature 😃

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

@Crunch09 anyway to get the indentation back to two spaces?

@Crunch09

This comment has been minimized.

Member

Crunch09 commented Jan 6, 2018

@ashmaroli Good point, there should be an option i think! I'll check

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

It also remove any comments left in the file 😕

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

The docs built fine locally so i guess the indentation is not that much of an issue for the list items (T.I.L.)

@Crunch09

This comment has been minimized.

Member

Crunch09 commented Jan 6, 2018

Yeah, if we want to have comments in there we need to stick with the regex i'm afraid :(

@ashmaroli Indentation is already set to 2 by default. I assume you meant the plugins and exclude arrays? I don't have enough YAML knowledge but i think there is no need for spaces so the YAML implementation doesn't add them.

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 6, 2018

Yes, I was referring to them.. but since everything works fine, guess its okay then.. just that the config file looks a bit crowded to me now..

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 6, 2018

This file is rarely edited so I guess it's OK.

@jekyllbot: merge +site

@jekyllbot jekyllbot merged commit 6a82754 into master Jan 6, 2018

3 checks passed

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

@jekyllbot jekyllbot deleted the read_config_file_as_yaml branch Jan 6, 2018

@DirtyF DirtyF restored the read_config_file_as_yaml branch Jan 6, 2018

@DirtyF DirtyF deleted the read_config_file_as_yaml branch Jan 6, 2018

localheinz added a commit to localheinz/jekyll that referenced this pull request Jan 7, 2018

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