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.

Sign up for GitHub

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

Jump to bottom

Fail gracefully if "sass" gem cannot be loaded #6573

Merged
merged 1 commit into from
Nov 21, 2017
Merged

Fail gracefully if "sass" gem cannot be loaded #6573

merged 1 commit into from
Nov 21, 2017

Conversation

ashmaroli
Copy link
Member

Closes #6423

Print our custom error message and let the user know they need to install "sass" gem instead of showing them the backtrace.

@pathawks
Copy link
Member

@ashmaroli Wow, nice sleuthing 🔍

@ashmaroli
Copy link
Member Author

Thank you 😃

DirtyF
DirtyF approved these changes Nov 21, 2017
View reviewed changes
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

Good catch!

@pathawks
Copy link
Member

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit c2586bb into jekyll:master Nov 21, 2017
jekyllbot added a commit that referenced this pull request Nov 21, 2017
@jekyllbot
Update history to reflect merge of #6573 [ci skip]
f6cf8b9
@pathawks
Copy link
Member

Maybe we should backport this to 3.6.x and cut a new release.

@ashmaroli Any desire to write a short announcement post about fixing this?

@ashmaroli
Copy link
Member Author

IMO, this doesn't warrant a separate release. The only change is that Jekyll is going to output the usual friendly-error message telling the user that they don't have sass installed, instead of the backtrace.
Actually, this issue is just an edge-case because, for >90% of users, sass will be installed automatically by bundle install because of it being a second-order runtime dependency.

jekyll < jekyll-sass-converter < sass

Regarding the associated issue ticket, there's every chance that Jekyll on Gentoo build still fails, (I've no idea why Bundler did not include sass in the build nor why it did not install sass if it wasn't installed in the system, to begin with..) I chose to close it with this because, I feel this is all that can be done from our side.

Any lastly, why backport this to just 3.6.x, when the bug can be traced to much older releases..?

@ashmaroli ashmaroli deleted the gracefully-require-sass branch November 22, 2017 02:23
@pathawks
Copy link
Member

Ok.

DirtyF pushed a commit that referenced this pull request Dec 7, 2017
@ashmaroli @DirtyF
fail gracefully if "sass" gem cannot be loaded (#6573)
6b003a4 
Merge pull request 6573
DirtyF pushed a commit that referenced this pull request Dec 7, 2017
@jekyllbot @DirtyF
Update history to reflect merge of #6573 [ci skip]
398e7d5
@jekyll jekyll locked and limited conversation to collaborators Jul 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DirtyF DirtyF

@pathawks pathawks

Assignees
No one assigned
Labels
fix frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

jekyll on gentoo fails
4 participants
@ashmaroli @pathawks @DirtyF @jekyllbot