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

FIX Update oembed config #107

Merged
merged 1 commit into from
May 12, 2022

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented May 11, 2022

Issue silverstripe/silverstripe-framework#10305

Requires silverstripe/silverstripe-framework#10312 to allow multiple backtick values to be set at once

Requires silverstripe/silverstripe-framework#10311 so that guzzle is being used

Release 2.10.1 2.11.0-beta1 when merged

@GuySartorelli
Copy link
Member

The fact that this is passing in travis when its dependencies aren't merged in yet seems a bit concerning... is there some tests that should be added to test this?

@emteknetnz emteknetnz changed the base branch from 2.10 to 2 May 11, 2022 03:30
@emteknetnz emteknetnz force-pushed the pulls/2.10/guzzle branch from 53f8329 to 5cfe51e Compare May 11, 2022 04:14
@emteknetnz
Copy link
Member Author

emteknetnz commented May 11, 2022

It's hard to test since what the purpose of this is to work on CWP hosting where there are some environment variable set.

I've added a unit test to test that framework is configured to create a Guzzle client, so in case we change the client again in the future so the config makes no sense, it should break.

Note: unit test needs silverstripe/silverstripe-framework#10311 merged first

Update: have tested this works CWP hosting in conjuction with the 2x framework PRs

@emteknetnz emteknetnz marked this pull request as ready for review May 11, 2022 04:23
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

The tests are failing

@emteknetnz emteknetnz force-pushed the pulls/2.10/guzzle branch from 5cfe51e to c6e03df Compare May 12, 2022 21:36
@emteknetnz emteknetnz force-pushed the pulls/2.10/guzzle branch from c6e03df to f694530 Compare May 12, 2022 21:52
@GuySartorelli GuySartorelli merged commit e381c5f into silverstripe:2 May 12, 2022
@GuySartorelli GuySartorelli deleted the pulls/2.10/guzzle branch May 12, 2022 22:40
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