Skip to content

exactEdgeLength -> edgeLength#639

Merged
isaacbrodsky merged 1 commit into
uber:masterfrom
ajfriend:exactEdgeLength
Aug 16, 2022
Merged

exactEdgeLength -> edgeLength#639
isaacbrodsky merged 1 commit into
uber:masterfrom
ajfriend:exactEdgeLength

Conversation

@ajfriend

@ajfriend ajfriend commented Aug 15, 2022

Copy link
Copy Markdown
Collaborator

Fix a naming consistency issue. edgeLength now lines up with cellArea.

@coveralls

coveralls commented Aug 15, 2022

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 98.717% when pulling ea82e16 on ajfriend:exactEdgeLength into a210a4a on uber:master.

@nrabinowitz

nrabinowitz commented Aug 15, 2022

Copy link
Copy Markdown
Collaborator

I'm okay with this change - we'll need to update the bindings as well. For consistency, we ought to offer getEdgeLengthAvg* functions as well, but def not required for launch.

@ajfriend

ajfriend commented Aug 15, 2022

Copy link
Copy Markdown
Collaborator Author

I'm okay with this change - we'll need to update the bindings as well. For consistency, we ought to offer getEdgeLengthAvg* functions as well, but def not required for launch.

We do already have getHexagonEdgeLengthAvgM(). Is that what you mean?

@nrabinowitz

nrabinowitz commented Aug 15, 2022

Copy link
Copy Markdown
Collaborator

I'm okay with this change - we'll need to update the bindings as well. For consistency, we ought to offer getEdgeLengthAvg* functions as well, but def not required for launch.

We do already have getHexagonEdgeLengthAvgM(). Is that what you mean?

Yes, nvm 🤦‍♂️

isaacbrodsky
isaacbrodsky approved these changes Aug 16, 2022
View reviewed changes

@isaacbrodsky isaacbrodsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are other places in the docs the function rename needs to be noted. I'll address them in a PR to release the next RC.

@isaacbrodsky
isaacbrodsky merged commit e10530f into uber:master Aug 16, 2022
@ajfriend
ajfriend deleted the exactEdgeLength branch August 16, 2022 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nrabinowitz nrabinowitz nrabinowitz approved these changes
@isaacbrodsky isaacbrodsky isaacbrodsky approved these changes
@dfellis dfellis Awaiting requested review from dfellis

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ajfriend @coveralls @nrabinowitz @isaacbrodsky