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

iOS Wrapper #663

Merged
merged 10 commits into from
Nov 28, 2022
Merged

iOS Wrapper #663

merged 10 commits into from
Nov 28, 2022

Conversation

PaulRps
Copy link
Contributor

@PaulRps PaulRps commented Nov 24, 2022

  • Renamed ConnectMeVcx to VcxAPI
  • Added type to parameters on function setLogger in VcxLogger
  • Added/exposed new functions to VcxAPI (equalized like in wrappers/node/src/rustlib.ts) related to OOB, Proof, CredDef, Schema etc.
  • Created VcxWrapperCallbacks.(h/m) to organize and reduce the code of VcxAPI (old ConnectMeVcx) file
  • Organized some imports and cleaned some code
  • Moved some type definitions from VcxAPI (old ConnectMeVcx) to VcxTypes
  • Redefined VcxHandle type to be unsigned int
  • Redefined type of all handles (connection, proof etc.) parameters on input and completion functions, in VcxAPI, to NSUInteger
  • Replaced deprecated function vcx_connection_create_with_connection_request by vcx_connection_create_with_connection_request_v2 in libvcx.h and VcxAPI

PaulRps and others added 6 commits November 7, 2022 10:07
…bvcx.h

removed deprecated function vcx_connection_create_with_connection_request from libvcx.h
replaced deprecated function in VcxAPI.h
replaced NSInteger type to NSUInteger in VcxAPI.h
added type to parameter on setLogger function in VcxLogger.h
@PaulRps PaulRps requested a review from a team as a code owner November 24, 2022 18:45
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2022

Codecov Report

Merging #663 (d7855e7) into main (aeee2bc) will decrease coverage by 15.97%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main     #663       +/-   ##
===========================================
- Coverage   63.82%   47.85%   -15.98%     
===========================================
  Files         213      212        -1     
  Lines       20944    15450     -5494     
  Branches     4688     3008     -1680     
===========================================
- Hits        13368     7393     -5975     
- Misses       3840     5770     +1930     
+ Partials     3736     2287     -1449     
Flag Coverage Δ
unittests-aries-vcx 47.85% <ø> (-15.78%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...tocols/proof_presentation/prover/states/initial.rs 0.00% <0.00%> (-100.00%) ⬇️
...cols/proof_presentation/verifier/states/initial.rs 0.00% <0.00%> (-100.00%) ⬇️
aries_vcx/src/indy/test_utils.rs 0.00% <0.00%> (-94.08%) ⬇️
aries_vcx/src/handlers/discovery/mod.rs 0.00% <0.00%> (-93.55%) ⬇️
aries_vcx/tests/utils/scenarios.rs 0.00% <0.00%> (-81.87%) ⬇️
...vcx/src/handlers/revocation_notification/sender.rs 0.00% <0.00%> (-75.00%) ⬇️
aries_vcx/src/utils/provision.rs 0.00% <0.00%> (-73.69%) ⬇️
aries_vcx/tests/utils/devsetup_agent.rs 0.00% <0.00%> (-73.13%) ⬇️
aries_vcx/src/indy/credentials/mod.rs 0.00% <0.00%> (-72.10%) ⬇️
agency_client/src/api/onboarding.rs 0.00% <0.00%> (-67.86%) ⬇️
... and 115 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Patrik-Stas
Copy link
Contributor

hi @PaulRps I didn't go trough it in detail, but reading through description it seem pretty cool! Give us a few days of time to catch up on these changes, but we'll get this in.

Hi @kukgini can you also have a look at this PR and share what you think? Would be much appreciated!

@Patrik-Stas
Copy link
Contributor

In the meantime @PaulRps , can you make sure your commits are signed off? The DCO check is failing https://github.com/hyperledger/aries-vcx/pull/663/checks?check_run_id=9694983151

For DCO check to pass, esentially every contain following construct:

Signed-off-by: Your Name <your@email>

at the end of commit message, for example like on this commit here
34dba47
image

You can disregard failing

CI / publish-docker-libvcx (pull_request) 

we'll have to fix our CI for this one

@PaulRps
Copy link
Contributor Author

PaulRps commented Nov 24, 2022

Hi @Patrik-Stas, i saw @kukgini's commit, i can wait to be merged and resolve conflicts later. The changes Kukgini did, i did too. So, about the commit signing i'll handle soon.

@Patrik-Stas
Copy link
Contributor

@PaulRps oh I see!

However note, in the other comment I was referring to pomfar's commit #656

@kukgini
Copy link
Contributor

kukgini commented Nov 25, 2022

I beleive @PaulRps 's PR is more desirable direction And reflecting more API changes that have been made in the meantime. My PR is just to hide some unnecessary exposure and convert handle to a more abstract data type (NSNumber). Anyway, I still think that It would be better to use NSNumber than NSUInteger. Is it ok that I drop my PR and make another PR based on @PaulRps 's PR?

@PaulRps
Copy link
Contributor Author

PaulRps commented Nov 25, 2022

Hi guys, i just have fixed the commit signing error, i did not set my email in gpg key before, that was the problem. @kukgini i can change the type of handles to NSNumber, it's ok to me, in my flutter demo i'm using it.

@kukgini
Copy link
Contributor

kukgini commented Nov 28, 2022

@PaulRps I dropped my PR. I would appreciate it if you could change the data type to NSNumber.

…on responses int and uint in VcxWrapperCallbacks

Signed-off-by: PaulRps <ricardopaulo18@hotmail.com>
@PaulRps
Copy link
Contributor Author

PaulRps commented Nov 28, 2022

it's done, @kukgini.

@Patrik-Stas
Copy link
Contributor

Patrik-Stas commented Nov 28, 2022

Looks good to me @PaulRps this is nice cleanup and reorg!

Also happy to see contributors collaborating together :-)

@Patrik-Stas Patrik-Stas merged commit d7e013a into hyperledger:main Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants