Skip to content

Conversation

@andrehjr
Copy link
Contributor

Hi there 🙌

We're preparing for upgrading our app to Rails 6.1 and found out that GoodData-ruby does not allow it yet. So, on this patch, we allow activesupport 6.1 to be installed and its compatibility.

Requiring active_support/core_ext/hash/compact on Rails 6.1 causes an error. Also shouldn't be needed for Ruby 2.5+. Check https://github.com/rails/rails/blob/6-0-stable/activesupport/lib/active_support/core_ext/hash/compact.rb

I've tried running specs locally.. but most of them are not running because they are trying to hit your staging servers. So I'm not sure.. if this is all the work needed!

Ruby 2.5+ (required by Rails 6) provides Hash#compact and Hash#compact! natively, so requiring active_support/core_ext/hash/compact is no longer necessary. Requiring it will raise LoadError in Rails 6.1.
@ghost
Copy link

ghost commented Jan 19, 2021

Build failed (check pipeline).

@danh-ung
Copy link

@phong-nguyen-duy please review/merge.

@hung-nguyen-hoang
Copy link
Contributor

ok to test

@hung-nguyen-hoang
Copy link
Contributor

extended test

@ghost
Copy link

ghost commented Jan 28, 2021

Build succeeded (check-extended pipeline).

@ghost
Copy link

ghost commented Jan 28, 2021

@sangtm
Copy link

sangtm commented Feb 1, 2021

We've test the following LCM bricks with image tag be1e509:

  1. Release (on Redshift)
  2. Provisioning (on Redshift)
  3. Rollout (on ADS)
  4. Users_filter (on ADS)
  5. Users_rovisioning on Redshift datasource datasource.
  6. Help

And they are working fine

cc @sang-truong @hung-nguyen-hoang

@sangtm sangtm added the merge label Feb 1, 2021
@yenkins
Copy link

yenkins commented Feb 1, 2021

Sonar scan result

  • 1 CODE_SMELL

More detail, see in https://sonarqube-dev.intgdc.com/dashboard?id=gooddata-ruby-gate-PR1715

@ghost ghost removed the merge label Feb 1, 2021
@ghost ghost merged commit db87588 into gooddata:master Feb 1, 2021
@andrehjr andrehjr deleted the allow-rails-6-1 branch February 9, 2021 18:50
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants