Add mention of classifier-reborn for LSI #5811

Merged
merged 2 commits into from Jan 24, 2017

Conversation

Projects
None yet
6 participants
@nhoizey
Contributor

nhoizey commented Jan 22, 2017

classifier-reborn is mandatory for LSI since Jekyll 3.0

Add mention of classifier-reborn for LSI
classifier-reborn is mandatory for LSI since Jekyll 3.0
@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Jan 22, 2017

Member

Thanks @nhoizey 😉

It's not mentioned on the variables page either, but if you try to run a build with the --lsi option, Jekyll will ask you to install the gem.

$ bundle exec jekyll build --lsi
...
      Generating...
  Dependency Error: Yikes! It looks like you don't have classifier-reborn or one of its dependencies installed. In order to use Jekyll as currently configured, you'll need to install this gem. The full error message from Ruby is: 'cannot load such file -- classifier-reborn' If you run into trouble, you can find helpful resources at http://jekyllrb.com/help/!
             ERROR: YOUR SITE COULD NOT BE BUILT:
                    ------------------------------------
                    classifier-reborn
Member

DirtyF commented Jan 22, 2017

Thanks @nhoizey 😉

It's not mentioned on the variables page either, but if you try to run a build with the --lsi option, Jekyll will ask you to install the gem.

$ bundle exec jekyll build --lsi
...
      Generating...
  Dependency Error: Yikes! It looks like you don't have classifier-reborn or one of its dependencies installed. In order to use Jekyll as currently configured, you'll need to install this gem. The full error message from Ruby is: 'cannot load such file -- classifier-reborn' If you run into trouble, you can find helpful resources at http://jekyllrb.com/help/!
             ERROR: YOUR SITE COULD NOT BE BUILT:
                    ------------------------------------
                    classifier-reborn
docs/_docs/configuration.md
@@ -233,7 +233,7 @@ class="flag">flags</code> (specified on the command-line) that control them.
<tr class="setting">
<td>
<p class="name"><strong>LSI</strong></p>
- <p class="description">Produce an index for related posts.</p>
+ <p class="description">Produce an index for related posts. The <a href="http://www.classifier-reborn.com/">classifier-reborn</a> plugin must be used.</p>

This comment has been minimized.

@ashmaroli

ashmaroli Jan 23, 2017

Member

How about simply: "( Requires classifier-reborn plugin )" ?

@ashmaroli

ashmaroli Jan 23, 2017

Member

How about simply: "( Requires classifier-reborn plugin )" ?

@nhoizey

This comment has been minimized.

Show comment
Hide comment
@nhoizey

nhoizey Jan 23, 2017

Contributor

@DirtyF should I make also a PR for the variables page, then?

Or maybe should the --lsi option and site.related_posts variable be removed from the documentation, since they're not in Jekyll by default anymore?

Contributor

nhoizey commented Jan 23, 2017

@DirtyF should I make also a PR for the variables page, then?

Or maybe should the --lsi option and site.related_posts variable be removed from the documentation, since they're not in Jekyll by default anymore?

@oe

This comment has been minimized.

Show comment
Hide comment
@oe

oe Jan 23, 2017

Member

@nhoizey i believe it should be fine to edit both pages in the scope of this PR

Member

oe commented Jan 23, 2017

@nhoizey i believe it should be fine to edit both pages in the scope of this PR

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Jan 23, 2017

Member

@nhoizey I understand you feel it's kind of a false promise as if you install classifier-reborn without gsl, the indexation takes forever. For now, links and explanation are already of great value for users.

I don't think @jekyll/core intend to remove completely the --lsi option and related posts from the core for now, but following the 80/20 principle, this might be something that could be added to the Jekyll 4 roadmap. Not my decision here.

Member

DirtyF commented Jan 23, 2017

@nhoizey I understand you feel it's kind of a false promise as if you install classifier-reborn without gsl, the indexation takes forever. For now, links and explanation are already of great value for users.

I don't think @jekyll/core intend to remove completely the --lsi option and related posts from the core for now, but following the 80/20 principle, this might be something that could be added to the Jekyll 4 roadmap. Not my decision here.

@parkr

parkr approved these changes Jan 23, 2017

This PR looks good to me with the added link/note here.

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Jan 24, 2017

Member

@jekyllbot: merge +doc

Member

DirtyF commented Jan 24, 2017

@jekyllbot: merge +doc

@jekyllbot jekyllbot merged commit 4ced56d into jekyll:master Jan 24, 2017

2 checks passed

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

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

@nhoizey nhoizey deleted the nhoizey:patch-3 branch Jan 24, 2017

parkr added a commit that referenced this pull request Jan 26, 2017

Merge branch 'master' into release-3-4-0
* master: (39 commits)
  Update history to reflect merge of #5798 [ci skip]
  Update history to reflect merge of #5822 [ci skip]
  use logger.info
  run codeclimate after success
  Update history to reflect merge of #5819 [ci skip]
  Fixed inaccuracy in "Built-in permalink styles" docs [skip ci]
  Update history to reflect merge of #5802 [ci skip]
  Update history to reflect merge of #5811 [ci skip]
  Update history to reflect merge of #5690 [ci skip]
  Update history to reflect merge of #5815 [ci skip]
  Review CI pages
  Rework CI doc to include multiple providers.
  Update history to reflect merge of #5812 [ci skip]
  Add jekyll-ga plug-in
  Update configuration.md
  Add mention of classifier-reborn for LSI
  Update history to reflect merge of #5810 [ci skip]
  Got that diaper money?
  Added note about --blank flag
  Update history to reflect merge of #5797 [ci skip]
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment