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

Revert adding leading slash to S3 class names #893

Merged
merged 1 commit into from
Mar 2, 2017

Conversation

cedricziel
Copy link
Collaborator

Closes: #892

Q A
Branch? 1.0
Bug fix? yes
New feature? no
BC breaks? /no
Deprecations? no
Tests pass? yes/no/maybe
Fixed tickets #892
License MIT

The service definition for the S3 resolver was changed slightly and instantiation broke for others. This change reverts it.

@lsmith77 lsmith77 added the State: Reviewing This item is being reviewed to determine if it should be accepted. label Mar 2, 2017
@cedricziel
Copy link
Collaborator Author

Ping @Chrysweel , @plozmun can you test the bugfix?

@rvanlaarhoven
Copy link
Contributor

Thanks, this seems to resolve it

@Chrysweel
Copy link

Yes it resolve the problem.

But my question is .. why a commit added the / in definition class previously ?

@cedricziel
Copy link
Collaborator Author

Well, that's how bugs get introduced :)

Those kinds of refactorings will be much more stable when we raise the php version requirement and thus can use class constants.

@cedricziel cedricziel requested a review from robfrawley March 2, 2017 12:11
@robfrawley
Copy link
Collaborator

Sorry guys, those got caught in a regex fixing FQCN in tests (which fixes some IDEs).

@robfrawley robfrawley merged commit 4a4a0ac into liip:1.0 Mar 2, 2017
@lsmith77 lsmith77 removed the State: Reviewing This item is being reviewed to determine if it should be accepted. label Mar 2, 2017
@robfrawley
Copy link
Collaborator

@cedricziel Is there anything we can do test-wise to ensure this regression is never introduced in the future?

@robfrawley robfrawley removed their request for review March 2, 2017 20:05
@cedricziel cedricziel deleted the revert-s3-client branch March 2, 2017 20:08
@cedricziel
Copy link
Collaborator Author

@robfrawley hmm. I don't think it's worth the hassle. - we should much rather move on and rely on PHP for those in the future :)

Don't worry about it.

@cedricziel
Copy link
Collaborator Author

Ahh, @robfrawley - we need a bugfix release. Can you create one?

@robfrawley
Copy link
Collaborator

Yeah, on it.

@cedricziel
Copy link
Collaborator Author

Awesome! 🎉

@robfrawley robfrawley mentioned this pull request Mar 2, 2017
@robfrawley
Copy link
Collaborator

Live!

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