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

(GH-1115) Bump json_pure to ~> 2.5.1 on Ruby >= 2.7 #1124

Merged
merged 1 commit into from
Jul 29, 2021
Merged

(GH-1115) Bump json_pure to ~> 2.5.1 on Ruby >= 2.7 #1124

merged 1 commit into from
Jul 29, 2021

Conversation

sanfrancrisko
Copy link
Contributor

@sanfrancrisko sanfrancrisko commented Jul 13, 2021

@sanfrancrisko sanfrancrisko requested a review from a team July 13, 2021 13:01
@sanfrancrisko sanfrancrisko self-assigned this Jul 13, 2021
@codecov
Copy link

codecov bot commented Jul 13, 2021

Codecov Report

Merging #1124 (6d877d5) into main (18754e5) will not change coverage.
The diff coverage is n/a.

❗ Current head 6d877d5 differs from pull request most recent head 5279f5f. Consider uploading reports for the commit 5279f5f to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1124   +/-   ##
=======================================
  Coverage   91.24%   91.24%           
=======================================
  Files         137      137           
  Lines        5571     5571           
=======================================
  Hits         5083     5083           
  Misses        488      488           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c76a53...5279f5f. Read the comment docs.

@sanfrancrisko
Copy link
Contributor Author

sanfrancrisko commented Jul 13, 2021

Tests:

  • bundle install using Ruby < 2.7 installs json_pure 2.1.0
  • bundle install using Ruby >= 2.7 installs json_pure 2.5.1
  • Removes warnings when running with json_pure 2.5.1 on Ruby 2.7 (see below)
  • pdk validate metadata (using json_pure 2.5.1) does not flag issues for valid metadata.json
  • pdk validate metadata (using json_pure 2.5.1) flags invalid metadata.json

json_pure 2.5.1 on Ruby 2.7:

~/bin/pdk validate metadata
pdk (WARN): This module is compatible with an older version of PDK. Run `pdk update` to update it to your version of PDK.
pdk (INFO): Using Ruby 2.7.3
pdk (INFO): Using Puppet 7.8.0
┌ [✔] Running metadata validators ...
├── [✔] Checking metadata syntax (metadata.json tasks/*.json).
└── [✔] Checking module metadata style (metadata.json).

@sanfrancrisko sanfrancrisko marked this pull request as draft July 14, 2021 16:54
@sanfrancrisko sanfrancrisko requested a review from a team July 16, 2021 09:51
@da-ar da-ar marked this pull request as ready for review July 29, 2021 10:48
@da-ar da-ar merged commit 684429f into puppetlabs:main Jul 29, 2021
@RandomNoun7
Copy link

Hi Content Team. I don't think this PR actually worked for at least one use case. When specifying PDK as a dependency in a module's Gemfile, the version of json_pure added to the Bundle is still 2.1.0. I think the reason is that Rubygems doesn't know how to add conditional runtime dependencies. You can see in the Rubygems page for pdk that the json_pure dependency is still listed as 2.1.0

The Puppet team ran into this issue and had to use a workaround as well because of the Rubygems not doing conditionals problem https://tickets.puppetlabs.com/browse/PUP-10797

The only thing I know worked in my testing was to add a specific entry for json_pure to the Gemfile status specifically that it must be >= 2.3.0.

I also found that just removing PDK from my Gemfile worked, and I don't know why it was there in the first place, but the fact remains that I don't think Rubygems has picked up the fix in this PR as intended.

@sanfrancrisko sanfrancrisko deleted the PDK-1115/main/bump_json_pure branch October 14, 2021 16:51
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.

Update json_pure gem to latest
4 participants