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

Update #start_with? to match String#start_with signature #444

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

tpendragon
Copy link
Contributor

Faraday recently started expecting a string-like #start_with? signature, where you can pass multiple strings to test against and it returns true if any of them passes. We were passing an RDF Value to Faraday since the API was mostly consistent.

It seems like the intent is to have the same API as String#start_with?, so this replicates it.

@CLAassistant
Copy link

CLAassistant commented Jul 5, 2024

CLA assistant check
All committers have signed the CLA.

Faraday recently started expecting a string-like #start_with? signature,
where you can pass multiple strings to test against and it returns true
if any of them passes. We were passing an RDF Value to Faraday since the
API was mostly consistent.

It seems like the intent is to have the same API as String#start_with?,
so this replicates it.
@tpendragon
Copy link
Contributor Author

tpendragon commented Jul 5, 2024

I don't really know what's going on with the failed CI, some help would be great @gkellogg if you get a chance to look at this.

@coveralls
Copy link

Coverage Status

coverage: 91.739%. remained the same
when pulling 47192eb on tpendragon:fix_start_with
into 5e32ea6 on ruby-rdf:develop.

@gkellogg
Copy link
Member

gkellogg commented Jul 5, 2024

I don't really know what's going on with the failed CI, some help would be great @gkellogg if you get a chance to look at this.

Sometimes just rerunning the failed tests fixes it. All green now.

Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

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

Looks like an omission from long ago. Are their other method definitions that also need to be touched?

@tpendragon
Copy link
Contributor Author

Looks like an omission from long ago. Are their other method definitions that also need to be touched?

I'm not sure, this was just the one giving us problems in samvera/valkyrie#958

@gkellogg
Copy link
Member

gkellogg commented Jul 5, 2024

Looks like an omission from long ago. Are their other method definitions that also need to be touched?

I'm not sure, this was just the one giving us problems in samvera/valkyrie#958

That is, in fact, the only such method definition.

Thanks for the contribution, I'll get an updated release out including this shortly.

@gkellogg gkellogg merged commit ca8db65 into ruby-rdf:develop Jul 5, 2024
9 checks passed
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.

4 participants