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

[7.17] Add base64 as a dependency #2400

Merged
merged 2 commits into from
Jun 3, 2024
Merged

Conversation

Earlopain
Copy link
Contributor

@Earlopain Earlopain commented Jun 3, 2024

It is my understanding that this branch is still maintained. You know the drill:

.../elasticsearch-ruby/elasticsearch-transport/lib/elasticsearch/transport/client.rb:18: warning: base64 was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.4.0. Add base64 to your Gemfile or gemspec.

preview1 for 3.4 has been released so this is starting to become an actual issue.

Could also do the unpack/pack thing I did for 8.x but 🤷, this is less invasive (not that that change was invasive in the first place though)


Also add CI for Ruby 3.3

Requires an uri require in the spec since this raises on access to `URI` otherwise
@Earlopain
Copy link
Contributor Author

CI failed on 3.3 for some reason: https://github.com/elastic/elasticsearch-ruby/actions/runs/9350312476/job/25733564008 Not sure, maybe it got loaded throuh some dependency beforhand or I just got unlucky with load order or something. I ran locally with 3.3 and that was fine

Copy link
Member

@picandocodigo picandocodigo left a comment

Choose a reason for hiding this comment

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

Thanks again @Earlopain!

@picandocodigo picandocodigo merged commit 25ab13a into elastic:7.17 Jun 3, 2024
9 checks passed
@Earlopain Earlopain deleted the 7.17-base64 branch June 3, 2024 15:23
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.

2 participants