Skip to content

Conversation

@princejwesley
Copy link

bytes field is in wrong place. This PR fixes it.

@drzraf
Copy link
Collaborator

drzraf commented Jan 6, 2021

There is clearly a problem. bytes is defined in FlowFile while it should be in FlowChunk.
Another problems is the name. It's actually used by prepareXhrRequest as a blob parameter.
blob, payload, content all seem better wording for what this variable is for (the comment-doc could also clarify this).
@princejwesley : Are you willing to reroll the patch in such a way?

@AidasK
Copy link
Member

AidasK commented Jan 7, 2021

This is a bug! I think we can merge it since prepareXhrRequest was not touched by this mr

@drzraf
Copy link
Collaborator

drzraf commented Jan 7, 2021

Sure, but this one won't merge as-is against neither master nor v3. What do you think about renaming the variable ?

@AidasK
Copy link
Member

AidasK commented Jan 7, 2021

we can rename it in v3 branch and this fix should be made in a different commit. We can't rename it on master because it would be a BC

@AidasK
Copy link
Member

AidasK commented Jan 7, 2021

#328

@drzraf drzraf closed this Jan 11, 2021
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 13, 2021
drzraf pushed a commit that referenced this pull request Jan 14, 2021
Renamed to `payload` (cf #195 & #328)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants