Skip to content
This repository was archived by the owner on Jul 15, 2025. It is now read-only.

Kotlin friendly names#1

Merged
karllessard merged 6 commits into from
May 18, 2021
Merged

Kotlin friendly names #1

karllessard merged 6 commits into from
May 18, 2021

Conversation

@rnett
Copy link
Contributor

@rnettrnett commented May 15, 2021

Renames Shape.size to Shape.get and add a toListOrNull method. Was part of tensorflow/java#165.

@rnettrnett mentioned this pull request May 15, 2021
Open
Craigacp
Craigacp suggested changes May 17, 2021
View reviewed changes
Copy link
Contributor

@CraigacpCraigacp left a comment

Choose a reason for hiding this comment

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

Could you provide some rational for why the method is renamed? Does kotlin treat get methods specially, or is there some conflict with size?

@rnett
Copy link
ContributorAuthor

rnett commented May 17, 2021•
edited
Loading

Yeah, Kotlin will automatically treat get as an operator, and rather than adding an extension in the Kotlin API Karl suggested I just change it here. Avoids overloading the meaning of size, too.

Craigacp
Craigacp reviewed May 17, 2021
View reviewed changes
Craigacp
Craigacp approved these changes May 17, 2021
View reviewed changes
Copy link
Contributor

@CraigacpCraigacp left a comment

Choose a reason for hiding this comment

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

I think I'm fine with this, but I'll let @karllessard merge it in case he has comments.

karllessard
karllessard suggested changes May 18, 2021
View reviewed changes
@rnettrnett requested a review from karllessardMay 18, 2021 20:43
@karllessardkarllessard merged commit d2aed4a into tensorflow:mainMay 18, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@CraigacpCraigacpCraigacp approved these changes

@karllessardkarllessardkarllessard approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rnett@Craigacp@karllessard