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

Small documentation changes for optional field options #86

Conversation

lukebakken
Copy link
Contributor

No description provided.

@tomas-abrahamsson
Copy link
Owner

Thanks! I will fetch. I plan to amend the previous commit with your corrections, rather than add it as another correction. It means your name as committer will be lost, but I think the commit history looks better that way. Would that be ok with you? I'll mention in the commit message that you have contributed. (If not ok, I'll add your commit on top, no problems, no big deal either)

@lukebakken
Copy link
Contributor Author

That's no problem at all.

I'm running a bunch of Riak tests today using gpb and as of now it looks like all of the pieces are in place for erlang_protobuffs compatibility via the ebp_compatibility option. Thanks and I'll keep testing!

@tomas-abrahamsson
Copy link
Owner

Ok, cool!

@lukebakken
Copy link
Contributor Author

@tomas-abrahamsson did you not want to use these changes?

@tomas-abrahamsson
Copy link
Owner

Oh, yes, indeed, I do! Sorry for confusion. The following have happened: I merged and amended locally as per earlier discussion, thus my local branch, and the branch on github had diverged. At this point, I decided (perhaps erroneously, see below) that I'd scrap the branch on github, and I guess since this was now a PR for a non-existing branch, that github decided it meant that the PR was closed. Hm. It looks like I've hit some close button, but that's not the case. I just deleted the branch on github.

I realize now you might be using the branch, perhaps still, for testing epb_compatibility, and so I should probably have pushed the updated (and diverged) branch instead of deleting it... Will do that. Sorry. Thanks for notifying.

@tomas-abrahamsson tomas-abrahamsson force-pushed the optionally-decode-omitted-optionals-to-defaults branch from 4d6f032 to ac61ac0 Compare September 20, 2016 23:51
@tomas-abrahamsson
Copy link
Owner

Pushed, and now I could reopen the issue again!

The following has happened: I pushed this branch again to github, but once I've pushed a new release on master, I intend to close this PR (and delete the branch again)

I've pushed another branch, new-optionally-decode-omitted-optionals-to-defaults (sorry for names getting longer and longer), that branch is what I'm merging to master (locally so far), later to be push as a new release (plus more commits).

The new-optionally-... branch contains this PR plus an extra eunit test, compared to this optionally-... branch. I think that should be the only difference.

@tomas-abrahamsson
Copy link
Owner

This is now included in 3.26.0 (closing)

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.

None yet

2 participants