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

.sass-cache doesn't *always* land in options['source'] #6500

Merged
merged 2 commits into from Nov 1, 2017

Conversation

Projects
None yet
8 participants
@DirtyF

DirtyF approved these changes Oct 30, 2017

@DirtyF DirtyF requested a review from jekyll/core Oct 30, 2017

@Crunch09

👍

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 31, 2017

Member

I think it's a bug that we don't write into the site source. I think that bug should be fixed in jekyll-sass-converter: jekyll/jekyll-sass-converter#64.

Member

parkr commented Oct 31, 2017

I think it's a bug that we don't write into the site source. I think that bug should be fixed in jekyll-sass-converter: jekyll/jekyll-sass-converter#64.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Oct 31, 2017

Contributor

TBQF, I would consider what you want a bug, my source is my source, if I change my source to say "source" and it's not the current directory, I would be pretty mad that source is being written to by Jekyll instead of the current directory, because that source directory was created to reduce the amount of clutter that Jekyll likes to create.

This is also part of a larger long-standing issue where Jekyll hasn't created a caching directory that can be customized, and is available to everyone, including plugins.

Contributor

envygeeks commented Oct 31, 2017

TBQF, I would consider what you want a bug, my source is my source, if I change my source to say "source" and it's not the current directory, I would be pretty mad that source is being written to by Jekyll instead of the current directory, because that source directory was created to reduce the amount of clutter that Jekyll likes to create.

This is also part of a larger long-standing issue where Jekyll hasn't created a caching directory that can be customized, and is available to everyone, including plugins.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Oct 31, 2017

Member

This is also part of a larger long-standing issue where Jekyll hasn't created a caching directory that can be customized

Absolutely, this is ultimately the path forward. For now though, I think it's best that we keep the SASS cache inside the source directory, as that is where we write all other cache files (.jekyll-metadata) and where I as a user would expect the cache to be.

Also, it seems like the SASS generator should be responsible for cleaning its own cache. Maybe we need to add a hook to make that possible?

Member

pathawks commented Oct 31, 2017

This is also part of a larger long-standing issue where Jekyll hasn't created a caching directory that can be customized

Absolutely, this is ultimately the path forward. For now though, I think it's best that we keep the SASS cache inside the source directory, as that is where we write all other cache files (.jekyll-metadata) and where I as a user would expect the cache to be.

Also, it seems like the SASS generator should be responsible for cleaning its own cache. Maybe we need to add a hook to make that possible?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Nov 1, 2017

Contributor

we keep the SASS cache inside the source directory

The SASS cache directory isn't and never does land within the source directory so changing that behavior now breaks well established behavior. Behavior I was chastised for just recently. It lands within the current working directory and has since inception, it was just never noticed because people rarely adjust the source directory (apparently?)

As far as leaving the cache to SASS, I don't know that it should be that way. They don't necessarily have supporting functions to do that (I could be wrong, I've not browsed the source all that closely) I think Jekyll should still handle it since it's the one that wants the cache (but that's not true either technically since it's enabled by default.) I've stated my opinion but really I'm neutral on leaving this part of the topic.

Contributor

envygeeks commented Nov 1, 2017

we keep the SASS cache inside the source directory

The SASS cache directory isn't and never does land within the source directory so changing that behavior now breaks well established behavior. Behavior I was chastised for just recently. It lands within the current working directory and has since inception, it was just never noticed because people rarely adjust the source directory (apparently?)

As far as leaving the cache to SASS, I don't know that it should be that way. They don't necessarily have supporting functions to do that (I could be wrong, I've not browsed the source all that closely) I think Jekyll should still handle it since it's the one that wants the cache (but that's not true either technically since it's enabled by default.) I've stated my opinion but really I'm neutral on leaving this part of the topic.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Nov 1, 2017

Member

Maybe we should focus instead on a centralized cache that is available to all plugins, which Jekyll can destroy during jekyll clean

Member

pathawks commented Nov 1, 2017

Maybe we should focus instead on a centralized cache that is available to all plugins, which Jekyll can destroy during jekyll clean

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Nov 1, 2017

Member

Maybe we should focus instead on a centralized cache that is available to all plugins, which Jekyll can destroy during jekyll clean

I like this idea.

Member

mattr- commented Nov 1, 2017

Maybe we should focus instead on a centralized cache that is available to all plugins, which Jekyll can destroy during jekyll clean

I like this idea.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Nov 1, 2017

Member

never does land within the source directory so changing that behavior now breaks well established behavior.

Can we safely assume that no one (users and plugin developers alike) interacts with a .sass-cache manually or programmatically and hence its location can be silently switched to the intended place at source_dir? Or is that going overboard?

Member

ashmaroli commented Nov 1, 2017

never does land within the source directory so changing that behavior now breaks well established behavior.

Can we safely assume that no one (users and plugin developers alike) interacts with a .sass-cache manually or programmatically and hence its location can be silently switched to the intended place at source_dir? Or is that going overboard?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Nov 1, 2017

Member

We probably cannot safely assume that, so let’s accept this PR and move forward with a cache folder. Is there a proposal for that?

Member

parkr commented Nov 1, 2017

We probably cannot safely assume that, so let’s accept this PR and move forward with a cache folder. Is there a proposal for that?

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Nov 1, 2017

Member

so let’s accept this PR and move forward

👍

Member

ashmaroli commented Nov 1, 2017

so let’s accept this PR and move forward

👍

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Nov 1, 2017

Member

@jekyllbot: merge +minor

Member

DirtyF commented Nov 1, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit f72e2cc into jekyll:master Nov 1, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment