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

Fix deprecation warnings (#796) #797

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

KarlHeitmann
Copy link
Contributor

@KarlHeitmann KarlHeitmann commented Aug 9, 2024

Description

I've cloned this repo on my machine, after running bundle install, I executed the ./script/server command and I've got the same deprecation message described by me in #796 . The only difference is I used the next patch version of ruby: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux] in my local minima repo.

The cause of the error seems to be the SASS compiler (I guess this version I have in my Gemfile.lock: sass-embedded (1.77.8-x86_64-linux-gnu)), because the deprecation message told me to read this page: https://sass-lang.com/documentation/breaking-changes/mixed-decls/

The deprecation message warns the offending line can impact the appearance of a page in the browser when the CSS nesting module will be introduced in the future https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_nesting

Proposal

After reading the deprecation explanation, I took the decision to opt-out by moving both offenders before the nested rule.

Why? Because the caveat / edge case the CSS Working Group took in consideration on this issue: w3c/csswg-drafts#8738 and the example found on the main sass-lang web page linked above are completely different than the case we have in minima theme.

Note both code snippets have the same property assigned a different value inside the nested rule, and the line below the nested rule.

In our case, the property found below the nested rule is margin-left, and the property found inside the nested rule is margin-right. So I opted out, instead of opting in by using the & selector.

I don't have a strong opinion here, if you want to opt in, I can change the PR. But I've tested the output on my local host by visually inspecting and using the dev tools to check the right CSS rules are applying correctly by looking at the a.page-link element found in _includes/header.html on my browser.

Everything looks good with the changes I've made.

I'll copy the content of my Gemfile.lock file below:

Gemfile.lock

PATH
  remote: .
  specs:
    minima (3.0.0.dev)
      jekyll (>= 3.5, < 5.0)
      jekyll-feed (~> 0.9)
      jekyll-seo-tag (~> 2.1)

GEM
  remote: https://rubygems.org/
  specs:
    addressable (2.8.7)
      public_suffix (>= 2.0.2, < 7.0)
    bigdecimal (3.1.8)
    colorator (1.1.0)
    concurrent-ruby (1.3.3)
    em-websocket (0.5.3)
      eventmachine (>= 0.12.9)
      http_parser.rb (~> 0)
    eventmachine (1.2.7)
    ffi (1.17.0-x86_64-linux-gnu)
    forwardable-extended (2.6.0)
    google-protobuf (4.27.3-x86_64-linux)
      bigdecimal
      rake (>= 13)
    http_parser.rb (0.8.0)
    i18n (1.14.5)
      concurrent-ruby (~> 1.0)
    jekyll (4.3.3)
      addressable (~> 2.4)
      colorator (~> 1.0)
      em-websocket (~> 0.5)
      i18n (~> 1.0)
      jekyll-sass-converter (>= 2.0, < 4.0)
      jekyll-watch (~> 2.0)
      kramdown (~> 2.3, >= 2.3.1)
      kramdown-parser-gfm (~> 1.0)
      liquid (~> 4.0)
      mercenary (>= 0.3.6, < 0.5)
      pathutil (~> 0.9)
      rouge (>= 3.0, < 5.0)
      safe_yaml (~> 1.0)
      terminal-table (>= 1.8, < 4.0)
      webrick (~> 1.7)
    jekyll-feed (0.17.0)
      jekyll (>= 3.7, < 5.0)
    jekyll-sass-converter (3.0.0)
      sass-embedded (~> 1.54)
    jekyll-seo-tag (2.8.0)
      jekyll (>= 3.8, < 5.0)
    jekyll-watch (2.2.1)
      listen (~> 3.0)
    kramdown (2.4.0)
      rexml
    kramdown-parser-gfm (1.1.0)
      kramdown (~> 2.0)
    liquid (4.0.4)
    listen (3.9.0)
      rb-fsevent (~> 0.10, >= 0.10.3)
      rb-inotify (~> 0.9, >= 0.9.10)
    mercenary (0.4.0)
    pathutil (0.16.2)
      forwardable-extended (~> 2.6)
    public_suffix (6.0.1)
    rake (13.2.1)
    rb-fsevent (0.11.2)
    rb-inotify (0.11.1)
      ffi (~> 1.0)
    rexml (3.3.4)
      strscan
    rouge (4.3.0)
    safe_yaml (1.0.5)
    sass-embedded (1.77.8-x86_64-linux-gnu)
      google-protobuf (~> 4.26)
    strscan (3.1.0)
    terminal-table (3.0.2)
      unicode-display_width (>= 1.1.1, < 3)
    unicode-display_width (2.5.0)
    webrick (1.8.1)

PLATFORMS
  x86_64-linux

DEPENDENCIES
  bundler
  minima!

BUNDLED WITH
   2.4.20

@KarlHeitmann KarlHeitmann mentioned this pull request Aug 9, 2024
Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM!

@ashmaroli
Copy link
Member

Thanks @KarlHeitmann
@jekyllbot: merge +fix

@jekyllbot jekyllbot merged commit aad1430 into jekyll:master Aug 11, 2024
3 checks passed
jekyllbot added a commit that referenced this pull request Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants