Skip to content

Conversation

@localheinz
Copy link
Contributor

@localheinz localheinz commented Jan 17, 2017

This PR

  • removes the keys when generating the payload

Follows #76.
Fixes #100.

πŸ’β€β™‚οΈ My bad, thanks a ton for digging into it, @maxjacobson! I will make sure to add tests later, too.

@maxjacobson
Copy link
Contributor

Hey @localheinz thanks for looking into this!

Did some playing around in a php playground to confirm that this looks like it'll do the right thing: https://www.tehplayground.com/zbTW7R0p3HD408nB

Some real tests would be better but for now I'm satisfied :)

@maxjacobson maxjacobson merged commit cda8fa8 into codeclimate:master Jan 17, 2017
@localheinz
Copy link
Contributor Author

Thank you, @maxjacobson!

@localheinz localheinz deleted the fix/array branch January 17, 2017 11:24
@localheinz
Copy link
Contributor Author

Let's tag this, too, what do you think?

This will help at least all the folks who have required the latest version, or use the ^ operator (despite the fact that it is an unstable package)!

@maxjacobson
Copy link
Contributor

@localheinz I'm open to it, but think we have some more work to do with our release process. Are you pulling in the test reporter at a specific tag when you use it? I sort of think everyone is just pulling in the latest master.

Some context: #54 isn't closed yet. #61 was merged, but we haven't perfectly adopted its recommendations - note that our releases don't include any downloads.

@localheinz
Copy link
Contributor Author

@maxjacobson

According to http://semver.org/#spec-item-4

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

I never require unstable dependencies using the ^ or any other wildcard operators - I always pin to a concrete version.

@maxjacobson
Copy link
Contributor

Yup, good call. Tagging 0.4.2 today #102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants