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

Sort gems #5746

Merged
merged 4 commits into from Jan 10, 2017

Conversation

Projects
None yet
5 participants
@DirtyF
Member

DirtyF commented Jan 9, 2017

À la Rubocop.

/cc @jekyll/core

DirtyF added some commits Jan 9, 2017

@parkr

parkr approved these changes Jan 9, 2017

Suuure!

@ashmaroli

one lone comment.. 😉

Show outdated Hide outdated jekyll.gemspec
s.add_runtime_dependency("jekyll-sass-converter", "~> 1.0")
s.add_runtime_dependency("jekyll-watch", "~> 1.1")
s.add_runtime_dependency("kramdown", "~> 1.3")

This comment has been minimized.

@ashmaroli

ashmaroli Jan 10, 2017

Member

before: the whitespace after gem-name improved readability

gemsp

after: it just looks weird..

gemspe

suggestion: normalize whitespace and improve readability. 😃

@ashmaroli

ashmaroli Jan 10, 2017

Member

before: the whitespace after gem-name improved readability

gemsp

after: it just looks weird..

gemspe

suggestion: normalize whitespace and improve readability. 😃

This comment has been minimized.

@DirtyF

DirtyF Jan 10, 2017

Member

done

@DirtyF
@oe

oe approved these changes Jan 10, 2017

@oe

This comment has been minimized.

Show comment
Hide comment
@oe

oe Jan 10, 2017

Member

@jekyllbot: merge +minor

Member

oe commented Jan 10, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 8a02606 into jekyll:master Jan 10, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

jekyllbot added a commit that referenced this pull request Jan 10, 2017

@DirtyF DirtyF deleted the DirtyF:sort-gems branch Jan 10, 2017

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 10, 2017

Member

@quiltbag For future reference, this probably should have been +dev or +site, as it doesn't affect any of Jekyll's runtime code (lib/).

Member

parkr commented Jan 10, 2017

@quiltbag For future reference, this probably should have been +dev or +site, as it doesn't affect any of Jekyll's runtime code (lib/).

@oe

This comment has been minimized.

Show comment
Hide comment
@oe

oe Jan 10, 2017

Member

@parkr okay! do we have a reference for those flags somewhere?

Member

oe commented Jan 10, 2017

@parkr okay! do we have a reference for those flags somewhere?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 10, 2017

Member

@quiltbag 😄 General docs are here: http://jekyllrb.com/docs/maintaining/ and for this specifically, http://jekyllrb.com/docs/maintaining/merging-a-pull-request/#merging – no big deal, I just updated it in 2236889.

Member

parkr commented Jan 10, 2017

@quiltbag 😄 General docs are here: http://jekyllrb.com/docs/maintaining/ and for this specifically, http://jekyllrb.com/docs/maintaining/merging-a-pull-request/#merging – no big deal, I just updated it in 2236889.

@oe

This comment has been minimized.

Show comment
Hide comment
@oe

oe Jan 10, 2017

Member

@parkr thanks :o i didn't see that doc

Member

oe commented Jan 10, 2017

@parkr thanks :o i didn't see that doc

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 10, 2017

Member

@quiltbag No problem at all! Feel free to reach out with any questions.

Member

parkr commented Jan 10, 2017

@quiltbag No problem at all! Feel free to reach out with any questions.

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