Skip to content
This repository has been archived by the owner on Dec 16, 2020. It is now read-only.

Add a bool return from onConfigure and add onValidateConfiguration(). #190

Merged
merged 8 commits into from
Sep 18, 2019

Conversation

jplevyak
Copy link
Contributor

#183

Signed-off-by: John Plevyak [email protected]

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: John Plevyak <[email protected]>
@jplevyak
Copy link
Contributor Author

@mandarjog

@jplevyak
Copy link
Contributor Author

FYI: as a sideeffect of me improving my dev environment to auto-format files, the API file is now being formatted in Envoy style. If this makes the review too difficult tell me and I'll do that separately.

@jplevyak
Copy link
Contributor Author

Ping

source/extensions/common/wasm/null/null_plugin.cc Outdated Show resolved Hide resolved
source/extensions/common/wasm/wasm.h Show resolved Hide resolved
source/extensions/common/wasm/wasm.cc Outdated Show resolved Hide resolved
api/wasm/cpp/proxy_wasm_api.h Outdated Show resolved Hide resolved
Signed-off-by: John Plevyak <[email protected]>
@@ -3,9 +3,10 @@

#include "proxy_wasm_intrinsics.h"

extern "C" EMSCRIPTEN_KEEPALIVE void proxy_onConfigure(uint32_t, int bad, char* configuration,
int size) {
extern "C" EMSCRIPTEN_KEEPALIVE uint32_t proxy_onConfigure(uint32_t, int bad, char* configuration,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you could have left it as void proxy_onConfigure(uint32_t, char* configuration, int size), since it's supposed to be bad signature anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but does it reverts the approval if I change anything... ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably.

source/extensions/common/wasm/wasm.h Show resolved Hide resolved
@jplevyak jplevyak merged commit 8c06645 into envoyproxy:master Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants