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

Replace sass-rails gem by sassc-rails #3286

Merged
merged 1 commit into from
Oct 22, 2019

Conversation

PierreMesure
Copy link
Collaborator

@PierreMesure PierreMesure commented Feb 10, 2019

References

Objectives

  • Replacing sass-rails by its C version, the former relying on a deprecated gem.

Notes

  • DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal in Sass 4.0. Use call(get-function("shake")) instead.

Gemfile Outdated Show resolved Hide resolved
@PierreMesure
Copy link
Collaborator Author

One failed spec so far:

1) Legislation Proposals Random pagination Random order maintained with pagination
     Failure/Error: expect(first_page_proposals_order & legislation_proposals_order).to eq([])
     
       expected: []
            got: ["legislation_proposal_24", "legislation_proposal_24_votes", "legislation_proposal_26", "legislation_proposal_26_votes"]
     
       (compared using ==)
     # ./spec/features/legislation/proposals_spec.rb:68:in `block (3 levels) in <top (required)>'

@PierreMesure
Copy link
Collaborator Author

OK, seems like it's just a flaky.

@voodoorai2000
Copy link
Member

Thanks @milovanderlinden, @PierreMesure!

We are still analysing the system dependencies that this gem has and the consequences for forks. We'll come back to it 👍

@javierm javierm mentioned this pull request Sep 10, 2019
@javierm javierm added dependencies Pull requests that updates a dependency Maintenance and removed Tech dependencies Pull requests that updates a dependency labels Sep 20, 2019
@javierm
Copy link
Member

javierm commented Oct 20, 2019

Just for the record, the reason we haven't approved this change yet is it broke the installation in the system we tried it (Madrid's preproduction server). It looks like the required version of GCC was not installed on that server, as mentioned in AyuntamientoMadrid#70.

So we need to consider some CONSUL installations will crash if we use sassc 🤔, although sooner or later this change will be inevitable.

On the other hand, distributions we officially support would not be affected...

@javierm javierm added this to Reviewing in Roadmap via automation Oct 22, 2019
@javierm
Copy link
Member

javierm commented Oct 22, 2019

After investigating this topic, it looks like everything will be fine with systems with GCC >= 4.7 (released in 2012). Madrid is probably the only place where this will be an issue, since they've got GCC 4.4.

@PierreMesure Could you rebase this pull request and use double quotes instead of single quotes? 🙏 After that, I think it'll be safe to merge it.

@javierm javierm moved this from Reviewing to Doing in Roadmap Oct 22, 2019
@PierreMesure
Copy link
Collaborator Author

Done.

@javierm javierm moved this from Doing to Reviewing in Roadmap Oct 22, 2019
Roadmap automation moved this from Reviewing to Testing Oct 22, 2019
Copy link
Member

@javierm javierm left a comment

Choose a reason for hiding this comment

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

@PierreMesure Thank you very much! 🎉

@javierm javierm merged commit ef6c7ee into consuldemocracy:master Oct 22, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Oct 22, 2019
@PierreMesure PierreMesure deleted the replace-sass-by-sassc branch October 23, 2019 04:45
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
…s-by-sassc

Replace sass-rails gem by sassc-rails
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

Ruby Sass is Deprecated
4 participants