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

Adjust gemspec to allow use with Jekyll 4.0 #492

Merged
merged 6 commits into from
Sep 25, 2019
Merged

Adjust gemspec to allow use with Jekyll 4.0 #492

merged 6 commits into from
Sep 25, 2019

Conversation

zocoi
Copy link
Contributor

@zocoi zocoi commented Aug 26, 2019

Since jejyll 4 is released, bump up the requirement version

@ashmaroli
Copy link
Member

@zocoi Couple of comments (FYI):

  • Dependency "jekyll", "~> 4" means the next release can't be run with Jekyll 3.x. That would be bad as the majority of the users would still be using Jekyll 3.x series.
    Instead opt for "jekyll", ">= 3.3", "< 5.0"
  • IMO, it'd be better if Travis would use the lowest common Ruby version which is rvm 2.4

@zocoi
Copy link
Contributor Author

zocoi commented Aug 27, 2019

@ashmaroli Agree with the comment, I also updated node to 8 which the oldest LTS version.

Since jekyll 4 introduces new Rubocop configs and this gem inherits from those cops. I updated the codebase to clear all the offenses.

@tosbourn
Copy link

Just heard about this project and would love to see this change make it in so I can try jekyll-admin on my blog. 💜

@mertkahyaoglu
Copy link
Member

@ashmaroli I think this is ready to go, right?

@ashmaroli
Copy link
Member

I think this is ready to go

Not quite. First, I shall merge #497. Then reduce the scope of changes in this PR..

@mertkahyaoglu
Copy link
Member

Good 👍 @zocoi We can merge this after conflicts are resolved then.

@ashmaroli
Copy link
Member

@zocoi I've thinned out the changes made in this branch to keep the PR focused to the primary intention.
Regarding the use of Node 8 for testing, we can bump if necessary in the future. For now, tests run on it just fine.

@ashmaroli ashmaroli changed the title Require newer jekyll gem version Adjust gemspec to allow use with Jekyll 4.0 Sep 25, 2019
Copy link
Member

@mertkahyaoglu mertkahyaoglu left a comment

Choose a reason for hiding this comment

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

Tested locally. Seems to be all good!

@mertkahyaoglu mertkahyaoglu mentioned this pull request Sep 25, 2019
@ashmaroli ashmaroli merged commit ac28fcd into jekyll:master Sep 25, 2019
@ashmaroli
Copy link
Member

Thank you for your contribution @zocoi 😃

@zocoi
Copy link
Contributor Author

zocoi commented Sep 25, 2019

you're welcome, thanks for responding to this PR quickly :)

@zocoi zocoi deleted the patch-1 branch September 25, 2019 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants