Skip to content

Add Zero and IsZero properties to Quaternion.#57354

Closed
Takym wants to merge 5 commits into
dotnet:mainfrom
Takym:Quaternion
Closed

Add Zero and IsZero properties to Quaternion.#57354
Takym wants to merge 5 commits into
dotnet:mainfrom
Takym:Quaternion

Conversation

@Takym

@Takym Takym commented Aug 13, 2021

Copy link
Copy Markdown
Contributor

This pull request resolves #57253.

@ghost ghost added area-System.Numerics community-contribution Indicates that the PR has been added by a community member labels Aug 13, 2021
@ghost

ghost commented Aug 13, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

This pull request resolves #57253.

Author: Takym
Assignees: -
Labels:

area-System.Numerics

Milestone: -

@GrabYourPitchforks

Copy link
Copy Markdown
Member

Hi @Takym! Thanks for the PR. Unfortunately the original issue is not yet marked api-approved, so we can't accept this PR at this time. See https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md#pull-requests for more information on the API approval process and the timelines for when PRs are accepted.

@Takym

Takym commented Aug 13, 2021

Copy link
Copy Markdown
Contributor Author

Hello @GrabYourPitchforks! Thank you for replying. Do I need to re-create a PR when the original issue is marked as api-approved, or will you reopen this PR?

@GrabYourPitchforks

Copy link
Copy Markdown
Member

I think you have the power to reopen the PR yourself at the right time. If not, leave a comment here and somebody should be able to reopen it on your behalf. The system won't auto-lock the issue for quite some time.

@Takym

Takym commented Aug 15, 2021

Copy link
Copy Markdown
Contributor Author

I clearly understood. Thanks.

@Takym

Takym commented Aug 17, 2021

Copy link
Copy Markdown
Contributor Author

Would you please reopen this PR because the issue was marked as api-approved?

@GrabYourPitchforks

GrabYourPitchforks commented Aug 17, 2021

Copy link
Copy Markdown
Member

@Takym Reopening. Only the Zero property was approved in API review; there didn't seem to be much love for an IsZero property.

Don't forget to update the ref assemblies and provide unit tests for the new API while you're at it. :)

Edit: Looks like you already beat me to the unit tests!

@Takym

Takym commented Aug 17, 2021

Copy link
Copy Markdown
Contributor Author

I added property definitions to the reference assembly in 7a54468.

Also, I have created a "no IsZero" version below:
Takym/runtime@Quaternion...Quaternion2

@stephentoub

Copy link
Copy Markdown
Member

This can be closed now that #58406 has been opened, yes?

@Takym

Takym commented Sep 1, 2021

Copy link
Copy Markdown
Contributor Author

@stephentoub Yes.

@Takym Takym closed this Sep 1, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Numerics community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal]: Quaternion.Zero is missing

3 participants