Skip to content

ARROW-6785: [JS] Remove superfluous child assignment#5394

Closed
akre54 wants to merge 2 commits intoapache:masterfrom
akre54:patch-1
Closed

ARROW-6785: [JS] Remove superfluous child assignment#5394
akre54 wants to merge 2 commits intoapache:masterfrom
akre54:patch-1

Conversation

@akre54
Copy link
Copy Markdown
Contributor

@akre54 akre54 commented Sep 16, 2019

From type.mjs:
Screenshot 2019-09-16 15 15 45

TypeScript's data modifiers automatically create a this.children = children assignment.

I suggest matching the other code and just removing the modifier instead of removing the assignment.

@fsaintjacques fsaintjacques requested a review from trxcllnt October 2, 2019 14:55
@fsaintjacques
Copy link
Copy Markdown
Contributor

There's a lint error. I also added a js reviewer :)

Copy link
Copy Markdown
Contributor

@trxcllnt trxcllnt left a comment

Choose a reason for hiding this comment

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

Good catch, I think this was a copy-paste oversight when I had to reimplement the Map. Other than lint error LGTM 👍

Co-Authored-By: Paul Taylor <paul.e.taylor@me.com>
@wesm wesm changed the title [JS] Remove superfluous child assignment ARROW-6785: [JS] Remove superfluous child assignment Oct 3, 2019
@wesm wesm closed this in cc05a89 Oct 3, 2019
kszucs pushed a commit that referenced this pull request Oct 5, 2019
From `type.mjs`:
![Screenshot 2019-09-16 15 15 45](https://user-images.githubusercontent.com/931368/64986774-c77b8b00-d895-11e9-9ed7-7d101b283361.png)

TypeScript's data modifiers [automatically create a `this.children = children` assignment](http://www.typescriptlang.org/play/#code/KYDwDg9gTgLgBAYwDYEMDOa4GUZQK4IwA8AKnKDMAHYAmmA3nANoDWwAngFxxq4CWVAOYBdbgBEUMFCXZhgcAL5wAvHBRV2APnIhKtTBKky5pWcAB0OfIQA0cEtvoAoOK8QQqvazGgAKMHgARkh8CHBQwCg0HkjsiAAWfEg0EVTcAGJ8wMmkrBwQAGb2wppMwgCUcM5uNTx4clC+5QDcLrWuMIlo5giJyakqCUkp1K01Cm2uAcGhcILA8DBmAJI0TVXhC3hQVPZmlrgE8Oh7cgfezYqTcNMhYT5WAoLrjBEw27sABlZHRPQAJPROnxur1hqlzABbFBgXy+AqVZTaT6AgrmKgoSHABScVHmJZyBSfcrmABWEAEvk+dmJCgUmk+lwmNTAUAglEIwBoPCkMFmTCw7EhgQgSHxEEeQhIKBEgzhrPZEG4P0IiMc1xqviI6i0NzZPhJYP61EGVDwSCQY3am3eOz1ioFQpFYoe-ClMuEgwA5CqYF6ra4FOVfL7zAqfATgC0nAogA).

I suggest matching the other code and just removing the modifier instead of removing the assignment.

Closes #5394 from akre54/patch-1 and squashes the following commits:

ea97eba <Adam Krebs> Update js/src/type.ts
32a126e <Adam Krebs>  Remove superfluous child assignment

Authored-by: Adam Krebs <akre54@users.noreply.github.com>
Signed-off-by: Wes McKinney <wesm+git@apache.org>
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