Skip to content

Box2<T> Box3<T> #194

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 17, 2020
Merged

Box2<T> Box3<T> #194

merged 10 commits into from
Jun 17, 2020

Conversation

HurricanKai
Copy link
Member

See #190

@HurricanKai HurricanKai changed the title [WIP] Box2<T> Box3<T> Box2<T> Box3<T> Jun 17, 2020
@HurricanKai HurricanKai marked this pull request as ready for review June 17, 2020 20:43
Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

Approved to merge, but part of me thinks that now that the structs are now immutable, is there any point in having the name of the methods be past tense? The pure overload of Translate was only called Translated to distinguish it from the former, however seeing as we no longer have Translate we could probably just rename Translated to Translate. What do you think?

@HurricanKai
Copy link
Member Author

Agreed! Will change

@Perksey
Copy link
Member

Perksey commented Jun 17, 2020

Awesome :D

Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

Once again approved to merge. Did you want to make the other maths types immutable as part of this PR or a different PR?

@HurricanKai
Copy link
Member Author

See #195

@Perksey
Copy link
Member

Perksey commented Jun 17, 2020

Ahaha nevermind, you opened a new pr before i typed my comment

@Perksey Perksey merged commit a50ef33 into dotnet:feature/maths Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants