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

Change toxencryptsave API to never overwrite pass keys. #334

Merged
merged 1 commit into from Dec 30, 2017

Conversation

Projects
None yet
6 participants
@iphydf
Member

iphydf commented Dec 16, 2016

This change is Reviewable

@iphydf iphydf added this to the v0.2.0 milestone Dec 16, 2016

@zetok

This comment has been minimized.

Show comment
Hide comment
@zetok

zetok Dec 16, 2016

Formatting could be perhaps improved, but aside from that public API looks fine. :lgtm_strong:


Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


toxencryptsave/toxencryptsave.h, line 244 at r1 (raw file):

 *
 * The Tox_Pass_Key structure is hidden in the implementation. It can be created
 * using tox_pass_key_derive or tox_pass_key_derive_with_salt and must be deallocated using tox_pass_key_free.

Formatting? Also functions' args could be formatted.


Comments from Reviewable

zetok commented Dec 16, 2016

Formatting could be perhaps improved, but aside from that public API looks fine. :lgtm_strong:


Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


toxencryptsave/toxencryptsave.h, line 244 at r1 (raw file):

 *
 * The Tox_Pass_Key structure is hidden in the implementation. It can be created
 * using tox_pass_key_derive or tox_pass_key_derive_with_salt and must be deallocated using tox_pass_key_free.

Formatting? Also functions' args could be formatted.


Comments from Reviewable

@robinlinden

This comment has been minimized.

Show comment
Hide comment
@robinlinden

robinlinden Dec 16, 2016

Member

:lgtm_strong:


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

Member

robinlinden commented Dec 16, 2016

:lgtm_strong:


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

@iphydf

This comment has been minimized.

Show comment
Hide comment
@iphydf

iphydf Dec 22, 2016

Member

Assigned toxbot to avoid accidentally pushing this since it's green. Also to make it not green so I don't get distracted by green PRs that can't be merged.

Member

iphydf commented Dec 22, 2016

Assigned toxbot to avoid accidentally pushing this since it's green. Also to make it not green so I don't get distracted by green PRs that can't be merged.

@iphydf

This comment has been minimized.

Show comment
Hide comment
@iphydf

iphydf Jan 8, 2017

Member

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.


toxencryptsave/toxencryptsave.h, line 244 at r1 (raw file):

Previously, zetok wrote…

Formatting? Also functions' args could be formatted.

Formatting needs to be fixed in apidsl. For that, apidsl needs to become smarter and know more about the semantics of a comment. For now, I'd like to leave it like this to keep the .api file nice.


Comments from Reviewable

Member

iphydf commented Jan 8, 2017

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.


toxencryptsave/toxencryptsave.h, line 244 at r1 (raw file):

Previously, zetok wrote…

Formatting? Also functions' args could be formatted.

Formatting needs to be fixed in apidsl. For that, apidsl needs to become smarter and know more about the semantics of a comment. For now, I'd like to leave it like this to keep the .api file nice.


Comments from Reviewable

@robinlinden

This comment has been minimized.

Show comment
Hide comment
@robinlinden

robinlinden Jan 10, 2017

Member

Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


Comments from Reviewable

Member

robinlinden commented Jan 10, 2017

Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


Comments from Reviewable

@iphydf iphydf modified the milestone: v0.2.0 Jan 10, 2017

@SkyzohKey

This comment has been minimized.

Show comment
Hide comment
@SkyzohKey

SkyzohKey Jan 20, 2017

Is this PR ready-to-merge ?

SkyzohKey commented Jan 20, 2017

Is this PR ready-to-merge ?

@iphydf

This comment has been minimized.

Show comment
Hide comment
@iphydf

iphydf Jan 24, 2017

Member

It's an API break, so it's waiting for 0.2, which is a bit more than a month away.


Review status: 2 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

Member

iphydf commented Jan 24, 2017

It's an API break, so it's waiting for 0.2, which is a bit more than a month away.


Review status: 2 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

@SkyzohKey SkyzohKey added the PR:ready label Mar 21, 2017

@robinlinden

This comment has been minimized.

Show comment
Hide comment
@robinlinden

robinlinden Dec 30, 2017

Member

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


Comments from Reviewable

Member

robinlinden commented Dec 30, 2017

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


Comments from Reviewable

@robinlinden robinlinden merged commit d26f0eb into TokTok:master Dec 30, 2017

4 of 5 checks passed

ci/circleci CircleCI is running your tests
Details
code-review/reviewable 2/2 LGTMs
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@iphydf iphydf deleted the iphydf:tes-api2 branch Dec 30, 2017

@iphydf iphydf modified the milestones: v0.2.0-RC1, v0.2.0 Jan 14, 2018

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