Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

revert interface change #367

Merged
merged 1 commit into from
Feb 24, 2015

Conversation

mlebkowski
Copy link

You made a breaking change to an interface in a minor patch library fix (4.0.164.0.17). This fucks up any library implementing this interface. According to semantic versioning this should be a major version bump, so I suggest reverting this change.

@gfosco
Copy link
Contributor

gfosco commented Feb 24, 2015

That's really a breaking change?.. I don't mind removing this type hinting, but what did it break?

@SammyK
Copy link
Contributor

SammyK commented Feb 24, 2015

Ah yes, the send() method signature the FacebookHttpable interface changed so any class coded to that interface will now have a different method signature and will cause errors so it's def a BC. Nice catch @mlebkowski!

@yguedidi
Copy link
Contributor

@gfosco yes it's a BC break

gfosco added a commit that referenced this pull request Feb 24, 2015
@gfosco gfosco merged commit e7425cf into facebookarchive:4.0-dev Feb 24, 2015
@yguedidi
Copy link
Contributor

@SammyK I'm late lol!

@SammyK
Copy link
Contributor

SammyK commented Feb 24, 2015

Beat you to it by seconds! :D

@gfosco
Copy link
Contributor

gfosco commented Feb 24, 2015

Haven't run into this before with a tagged release... Should I tag this 4.0.18 or delete and re-tag 4.0.17?

@SammyK
Copy link
Contributor

SammyK commented Feb 24, 2015

I'd just tag 4.0.18 and count 4.0.17 as a bug release.

@gfosco
Copy link
Contributor

gfosco commented Feb 24, 2015

Moved too quickly, it's tagged 4.0.18 but the readme badge and the version string are still .17... (tableflip)

@SammyK
Copy link
Contributor

SammyK commented Feb 24, 2015

Lol

@gfosco
Copy link
Contributor

gfosco commented Feb 24, 2015

Fixed. Thanks all.

@yguedidi
Copy link
Contributor

👍

@mlebkowski
Copy link
Author

Thanks for a swift reaction! :D

@mlebkowski mlebkowski deleted the fix/breaking-changes branch February 24, 2015 19:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants