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

Node wrapper eslint fix anys #237

Merged
merged 8 commits into from
Dec 16, 2020
Merged

Conversation

OoDeLally
Copy link
Contributor

@OoDeLally OoDeLally commented Dec 14, 2020

These changes require a bit of attention, since they are not merely cosmetics

@OoDeLally OoDeLally requested a review from a team as a code owner December 14, 2020 14:33
Pascal Heitz added 4 commits December 14, 2020 16:01
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
@codecov-io
Copy link

codecov-io commented Dec 14, 2020

Codecov Report

Merging #237 (fe9895b) into master (824a6b2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   57.69%   57.69%           
=======================================
  Files         149      149           
  Lines       19902    19902           
  Branches     4464     4464           
=======================================
  Hits        11482    11482           
  Misses       5071     5071           
  Partials     3349     3349           
Flag Coverage Δ
integration 26.14% <ø> (ø)
unittests 51.95% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 824a6b2...fe9895b. Read the comment docs.

Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
}

// tslint:disable object-literal-sort-keys
export const FFIConfiguration: { [Key in keyof IFFIEntryPoint]: any } = {
export const FFIConfiguration = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the constraint removed? I think it was quite useful to protect from inconsistency / typos between IFFIEntryPoint and FFIConfiguration

wrappers/node/src/rustlib.ts Show resolved Hide resolved
wrappers/node/src/api/logging.ts Outdated Show resolved Hide resolved
@@ -215,16 +215,16 @@ export class Wallet {
Callback(
'void',
['uint32', 'uint32', 'pointer', 'uint32'],
(xHandle: number, err: number, details: any, length: number) => {
(xHandle: number, err: number, detailsPtr: ref.Type, length: number) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ref type is complex structure according to its definition

export interface Type {
    /** The size in bytes required to hold this datatype. */
    size: number;
    /** The current level of indirection of the buffer. */
    indirection: number;
    /** To invoke when `ref.get` is invoked on a buffer of this type. */
    get(buffer: Buffer, offset: number): any;
    /** To invoke when `ref.set` is invoked on a buffer of this type. */
    set(buffer: Buffer, offset: number, value: any): void;
    /** The name to use during debugging for this datatype. */
    name?: string;
    /** The alignment of this datatype when placed inside a struct. */
    alignment?: number;
}

whereas rust function looks like this

pub extern fn vcx_wallet_sign_with_address(command_handle: CommandHandle,
                                           payment_address: *const c_char,
                                           message_raw: *const u8,
                                           message_len: u32,
                                           cb: Option<extern fn(xcommand_handle: CommandHandle, err: u32,
                                                                signature: *const u8, signature_len: u32)>) -> u32 {

where signature: *const u8 in callback maps to detailsPtr and it's just u8 pointer.

We don't use signWithAddress now, probably not even testing it so I don't know if your change actually works or not, but doesn't seem correct to me.

Pascal Heitz added 2 commits December 15, 2020 14:45
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
Signed-off-by: Pascal Heitz <pascal.heitz@absa.africa>
@Patrik-Stas Patrik-Stas merged commit 82f5210 into master Dec 16, 2020
@Patrik-Stas Patrik-Stas deleted the node-wrapper-eslint-fix-anys branch December 16, 2020 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants