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.

Sign up for GitHub

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

Jump to bottom

external C API baseline - compat w/ liberasurecode #266

Merged
merged 1 commit into from
Feb 4, 2019
Merged

external C API baseline - compat w/ liberasurecode #266

merged 1 commit into from
Feb 4, 2019

Conversation

vrancurel
Copy link
Contributor

@vrancurel vrancurel commented Dec 7, 2018 •
edited
Loading

Fix #260
Fix #267

@vrancurel
Copy link
Contributor Author

vrancurel commented Dec 7, 2018 •
edited
Loading

@slaperche-scality what is the best practice to convert to clang-format ? thx
Also for manually passing the linter ?

@vrancurel
Copy link
Contributor Author

Please also review https://github.com/scality/liberasurecode/pull/1/files

@vrancurel vrancurel force-pushed the feat/external-c-api branch 4 times, most recently from 04fa15b to 53d8eb1 Compare December 7, 2018 18:02
slaperche-scality
slaperche-scality suggested changes Dec 14, 2018
View reviewed changes
src/CMakeLists.txt Outdated Show resolved Hide resolved
src/fec_base.h Outdated Show resolved Hide resolved
src/fec_base.h Outdated Show resolved Hide resolved
src/fec_base.h Show resolved Hide resolved
src/fec_base.h Outdated Show resolved Hide resolved
src/quadiron-c.cpp Outdated Show resolved Hide resolved
src/quadiron-c.cpp Outdated Show resolved Hide resolved
src/quadiron-c.cpp Outdated Show resolved Hide resolved
src/quadiron-c.cpp Outdated Show resolved Hide resolved
src/quadiron-c.cpp Outdated Show resolved Hide resolved
lamphamsy
lamphamsy reviewed Dec 14, 2018
View reviewed changes
Copy link
Contributor

@lamphamsy lamphamsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice. There are still some parts to improve.

src/fec_base.h Show resolved Hide resolved
src/property.h Outdated Show resolved Hide resolved
src/quadiron-c.cpp Outdated Show resolved Hide resolved
@vrancurel vrancurel added the Release 0.1 Initial Release label Dec 14, 2018
@vrancurel vrancurel force-pushed the feat/external-c-api branch 3 times, most recently from d6d6a14 to 3f77c8e Compare December 20, 2018 01:58
@vrancurel
Copy link
Contributor Author

@slaperche-scality addressed most of the comments. still need input for some.

@vrancurel vrancurel mentioned this pull request Dec 28, 2018
Closed
@vrancurel
Copy link
Contributor Author

addressed most of the concerns @slaperche-scality

vrancurel
vrancurel commented Dec 29, 2018
View reviewed changes
src/quadiron_c.cpp Outdated Show resolved Hide resolved
vrancurel
vrancurel commented Dec 29, 2018
View reviewed changes
src/quadiron_c.cpp Outdated Show resolved Hide resolved
@vrancurel vrancurel force-pushed the feat/external-c-api branch from a619e36 to 80d0014 Compare January 1, 2019 01:59
slaperche-scality
slaperche-scality reviewed Jan 2, 2019
View reviewed changes
src/quadiron_c.h Outdated Show resolved Hide resolved
src/quadiron_c.cpp Outdated Show resolved Hide resolved
src/quadiron_c.cpp Show resolved Hide resolved
@vrancurel vrancurel force-pushed the feat/external-c-api branch 2 times, most recently from e8e2b6a to 8248004 Compare January 11, 2019 19:43
@vrancurel vrancurel added the Standby It's waiting other works label Jan 12, 2019
@vrancurel
Copy link
Contributor Author

Do not merge, still writing unit tests for the C API ...

@vrancurel vrancurel force-pushed the feat/external-c-api branch from 8248004 to 21be50a Compare January 13, 2019 02:29
@vrancurel vrancurel force-pushed the feat/external-c-api branch 2 times, most recently from 514121c to f5291a7 Compare January 14, 2019 02:59
@vrancurel vrancurel removed the Standby It's waiting other works label Jan 14, 2019
@vrancurel
Copy link
Contributor Author

@slaperche-scality should be good for merge now

@vrancurel vrancurel force-pushed the feat/external-c-api branch 2 times, most recently from 398cb52 to e753927 Compare January 14, 2019 05:05
@vrancurel vrancurel force-pushed the feat/external-c-api branch from e753927 to 9cb3581 Compare January 30, 2019 02:11
slaperche-scality
slaperche-scality suggested changes Jan 30, 2019
View reviewed changes
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
slaperche-scality
slaperche-scality suggested changes Jan 30, 2019
View reviewed changes
Copy link
Contributor

@slaperche-scality slaperche-scality left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, my review is complete.

test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
@vrancurel vrancurel force-pushed the feat/external-c-api branch 3 times, most recently from c29ab20 to b6bbb28 Compare January 31, 2019 03:20
lamphamsy
lamphamsy approved these changes Jan 31, 2019
View reviewed changes
Copy link
Contributor

@lamphamsy lamphamsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice!

@vrancurel vrancurel force-pushed the feat/external-c-api branch from b6bbb28 to f0ed306 Compare February 1, 2019 02:21
@vrancurel
Copy link
Contributor Author

@slaperche-scality should be good now

slaperche-scality
slaperche-scality suggested changes Feb 1, 2019
View reviewed changes
Copy link
Contributor

@slaperche-scality slaperche-scality left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good to merge with the few change I suggested.

Nice PR, especially for the unit tests :)

test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
test/quadiron_c_utest.cpp Outdated Show resolved Hide resolved
@vrancurel vrancurel force-pushed the feat/external-c-api branch from f0ed306 to c153952 Compare February 3, 2019 01:33
@vrancurel vrancurel merged commit 6de0270 into master Feb 4, 2019
@vrancurel vrancurel deleted the feat/external-c-api branch February 4, 2019 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slaperche-scality slaperche-scality slaperche-scality approved these changes

@lamphamsy lamphamsy lamphamsy approved these changes

Assignees
No one assigned
Labels
Release 0.1 Initial Release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vrancurel @lamphamsy @slaperche-scality