Skip to content
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

Document System.Numerics APIs #2912

Merged
merged 10 commits into from Aug 6, 2019

Conversation

@carlossanlop
Copy link
Member

commented Aug 1, 2019

op_Addition
op_Division
op_Multiply
op_Substraction
Substract

Added a member group for each set and moved the remark there.

@carlossanlop carlossanlop added this to the August 2019 milestone Aug 1, 2019

@carlossanlop carlossanlop self-assigned this Aug 1, 2019

@carlossanlop carlossanlop added this to In progress in July 2019 via automation Aug 1, 2019

@Jozkee
Copy link
Member

left a comment

Is more a suggestion, but I think we could replace double for real since that is what we are trying to represent here.

xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved

@mairaw mairaw added this to In progress in August 2019 via automation Aug 2, 2019

@mairaw mairaw removed this from In progress in July 2019 Aug 2, 2019

@tannergooding

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

Overall LGTM. Left some comments on op_Addition, but it should also apply to the other operators

@carlossanlop carlossanlop requested review from Jozkee, mairaw and tannergooding Aug 2, 2019

@carlossanlop

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

@tannergooding @mairaw @Jozkee thanks for your suggestions. I applied them all, and added individual remarks for each specific overload, while keeping a generic MethodGroup remark as well. Let me know what you think.

@Jozkee
Copy link
Member

left a comment

Uhm, why did you copy the paragraphs in the remarks section back to their particular member? isn't that somewhat redundant?

Also, suggested a change to spell for example The Addition operator in singular rather than in plural.

xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
@rpetrusha
Copy link
Contributor

left a comment

I've left some comments and suggestions, @carlossanlop. The most significant is that the <AssemblyInfo> elements need to be added; without them, the overloaded member description doesn't appear in the built page.

xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Show resolved Hide resolved
xml/System.Numerics/Complex.xml Show resolved Hide resolved
xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
xml/System.Numerics/Complex.xml Show resolved Hide resolved

carlossanlop and others added some commits Aug 5, 2019

suggestions by rpetrusha and Jozkee
Co-Authored-By: David Cantu <jozkyy@gmail.com>
Co-Authored-By: Ron Petrusha <ronpet@microsoft.com>
@carlossanlop

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@rpetrusha can you please take a look at the last commit I submitted?

xml/System.Numerics/Complex.xml Outdated Show resolved Hide resolved
@rpetrusha

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

This looks good. Is it ready to merge, @carlossanlop?

@carlossanlop

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@rpetrusha If you're ok with the language from my last 2 commits then yes, it's ready to merge.

@rpetrusha

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

I'll merge it now, @carlossanlop.

@rpetrusha rpetrusha merged commit 35143de into dotnet:master Aug 6, 2019

6 checks passed

OpenPublishing.Build Validation status: passed
Details
OpenPublishing.Build (1 of 3) Waiting for processor completed at 10:15:26 PST
OpenPublishing.Build (2 of 3) Preparing completed at 10:27:24 PST
OpenPublishing.Build (3 of 3) Building completed at 10:53:30 PST
WIP Ready for review
Details
license/cla All CLA requirements met.
Details

August 2019 automation moved this from In progress to Done Aug 6, 2019

@carlossanlop carlossanlop deleted the carlossanlop:Numerics branch Aug 6, 2019

Jozkee added a commit to Jozkee/dotnet-api-docs that referenced this pull request Aug 6, 2019

mairaw added a commit that referenced this pull request Aug 6, 2019

Added documentation for System.Numerics.Complex APIs targeted for 3.0 (
…#2946)

* Adding documentation for System.Numerics APIs.

* Fixing typo missed in #2912

* Fixing syntax errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.