Skip to content
This repository has been archived by the owner on Nov 14, 2023. It is now read-only.

Make more fields public #28

Merged
merged 1 commit into from
Jun 20, 2016
Merged

Make more fields public #28

merged 1 commit into from
Jun 20, 2016

Conversation

aidanhs
Copy link
Collaborator

@aidanhs aidanhs commented Jun 16, 2016

This PR makes all fields in responses public - there's not much point in having them if they can't be used!

On the other hand, I was considering making a PR to convert a bunch of things to better types (e.g. f64 id to u64), which would be less of a breaking change (slightly) if these fields stayed private for now.

Thoughts @kbknapp ?

@yo-bot
Copy link
Collaborator

yo-bot commented Jun 16, 2016

r? @yberreby

(yo-bot has picked a reviewer for you, use r? to override)

@aidanhs
Copy link
Collaborator Author

aidanhs commented Jun 16, 2016

r? @kbknapp

@yo-bot yo-bot assigned kbknapp and unassigned yberreby Jun 16, 2016
@kbknapp
Copy link
Owner

kbknapp commented Jun 20, 2016

@aidanhs sounds good! Yeah those types that should be u64 vs f64 and the like is a good change too. I think I made them f64 to begin with since the DO documentation never stated whether they would be <0 or use decimals.

Since this is still pre 1.0 simply bumping the minor is all that needs to be done in order to make a breaking change.

@homu r+

@homu
Copy link
Collaborator

homu commented Jun 20, 2016

📌 Commit 7b54a33 has been approved by kbknapp

@homu
Copy link
Collaborator

homu commented Jun 20, 2016

⚡ Test exempted - status

@homu homu merged commit 7b54a33 into master Jun 20, 2016
homu added a commit that referenced this pull request Jun 20, 2016
Make more fields public

This PR makes all fields in responses public - there's not much point in having them if they can't be used!

On the other hand, I was considering making a PR to convert a bunch of things to better types (e.g. f64 id to u64), which would be less of a breaking change (slightly) if these fields stayed private for now.

Thoughts @kbknapp ?
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

5 participants