Skip to content

chore: es6 unmarshaller #108

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
May 5, 2020
Merged

chore: es6 unmarshaller #108

merged 2 commits into from
May 5, 2020

Conversation

grant
Copy link
Member

@grant grant commented May 1, 2020

No description provided.

@grant grant requested a review from lance May 1, 2020 17:48
@grant grant self-assigned this May 1, 2020
@grant
Copy link
Member Author

grant commented May 5, 2020

I added the suggested changes after you insisted, but they break the build.

@lance
Copy link
Member

lance commented May 5, 2020

You could have made the change locally and easily seen the syntax error and then pushed them. Were you trying to prove a point? I didn't insist. I just suggested.

@grant
Copy link
Member Author

grant commented May 5, 2020

You could have made the change locally and easily seen the syntax error and then pushed them. Were you trying to prove a point? I didn't insist. I just suggested.

The suggested change doesn't just introduce format errors which are easy to fix. It introduces a rogue "catch" statement. I don't understand what we're trying to do. I'm just trying to change the unmarshaller to es6, not change the file in this small PR which might require the tests to change.

Can we please approve the PR assuming we will revert the last commit?

@grant
Copy link
Member Author

grant commented May 5, 2020

There's also an error with a repeated "payload is null or undefined" error.

@grant grant force-pushed the grant_unmarshal branch 2 times, most recently from 46b4bcc to ea289a2 Compare May 5, 2020 19:33
@grant
Copy link
Member Author

grant commented May 5, 2020

OK, I fixed the error. The tests now pass locally.

@grant grant force-pushed the grant_unmarshal branch from ea289a2 to c722b03 Compare May 5, 2020 19:35
@lance
Copy link
Member

lance commented May 5, 2020

Grant, in a spirit of collaboration, I merely made a suggestion. Your veiled hostility is unwelcome.

@grant
Copy link
Member Author

grant commented May 5, 2020

OK, I just don't want suggestions of breaking the PR as it was perfectly fine before. There were 2 issues with the suggestion. It's hard enough to make PRs to this repo as is.

@grant grant force-pushed the grant_unmarshal branch 2 times, most recently from 5130527 to 1ef93d6 Compare May 5, 2020 19:46
@lance
Copy link
Member

lance commented May 5, 2020

And yet you continue with the finger pointing.

@grant
Copy link
Member Author

grant commented May 5, 2020

I'm trying to fix the DCO error, I think the commit from GitHub with the suggested change doesn't isn't signed off and is breaking DCO.

@grant grant force-pushed the grant_unmarshal branch from 1ef93d6 to 226604c Compare May 5, 2020 19:52
Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>

chore: fix lint multiline if

Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>

Update lib/bindings/http/unmarshaller.js

Co-authored-by: Lance Ball <lball@redhat.com>

fix: fix pr suggestion errors

Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>

refactor: fix DCO in PR

Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>

fix: fix pr suggestion errors

Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>
@grant grant force-pushed the grant_unmarshal branch from 226604c to bfd7f25 Compare May 5, 2020 19:54
Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>
@grant grant merged commit 79ec3ef into master May 5, 2020
@grant grant deleted the grant_unmarshal branch May 5, 2020 20:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants