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

upgrade comments of class constructors #47

Open
ghost opened this issue Dec 25, 2020 · 8 comments · May be fixed by #63
Open

upgrade comments of class constructors #47

ghost opened this issue Dec 25, 2020 · 8 comments · May be fixed by #63
Assignees

Comments

@ghost
Copy link

ghost commented Dec 25, 2020

it is necessary to improve the comments of class constructors.

@uselessgoddess
Copy link
Member

@Konard Now they are better?

/// <summary>
/// <para>Initializes a new instance of the Range class.</para>
/// <para>Инициализирует новый экземпляр класса Range.</para>
/// </summary>
/// <param name="minimumAndMaximum"><para>Single value for both Minimum and Maximum fields.</para><para>Одно значение для полей Minimum и Maximum.</para></param>

/// <summary>
/// <para>Initializes a new instance of the Range class.</para>
/// <para>Инициализирует новый экземпляр класса Range.</para>
/// </summary>
/// <param name="minimum"><para>The minimum value of the range.</para><para>Минимальное значение диапазона.</para></param>
/// <param name="maximum"><para>The maximum value of the range.</para><para>Максимальное значение диапазона.</para></param>
/// <exception cref="ArgumentException"><para>Thrown when the maximum is less than the minimum.</para><para>Выбрасывается, когда максимум меньше минимума.</para></exception>

@Konard
Copy link
Member

Konard commented Jul 27, 2021

@FreePhoenix888 @TwinkmrMask do you have any ideas how to make these comments better?

@FreePhoenix888
Copy link
Member

FreePhoenix888 commented Jul 27, 2021

 /// <summary> 
 /// <para>Initializes a <see cref="Range"/> instance.</para> 
 /// <para>Инициализирует экземпляр <see cref="Range"/>.</para> 
 /// </summary> 
 /// </summary> 
 /// <param name="minimum"><para>The minimum value of the range.</para><para>Минимальное значение диапазона.</para></param> 
 /// <param name="maximum"><para>The maximum value of the range.</para><para>Максимальное значение диапазона.</para></param> 
 /// <exception cref="ArgumentException"><para>The <paramref name="maximum"/> is less than the <paramref name="minumum"/>.</para><para><paramref name="maximum"/> меньше <paramref name="minumum"/>.</para></exception> 

Take an example from here. Check how they have written Exceptions. Shorter: remove Thrown when ... from your exception description, write only the condition when the exception throwns.
By the way I would recommend to read about recommended tags for docs.

@Konard
Copy link
Member

Konard commented Jul 27, 2021

 /// <summary> 
 /// <para>Initializes a <see cref="Range"/> instance.</para> 
 /// <para>Инициализирует экземпляр <see cref="Range"/>.</para> 
 /// </summary> 
 /// </summary> 
 /// <param name="minimum"><para>The minimum value of the range.</para><para>Минимальное значение диапазона.</para></param> 
 /// <param name="maximum"><para>The maximum value of the range.</para><para>Максимальное значение диапазона.</para></param> 
 /// <exception cref="ArgumentException"><para>The <paramref name="maximum"/> is less than the <paramref name="minumum"/>.</para><para><paramref name="maximum"/> меньше <paramref name="minumum"/>.</para></exception> 

Take an example from here. Check how they have written Exceptions. Shorter: remove Thrown when ... from your exception description, write only the condition when the exception throwns.
By the way I would recommend to read about recommended tags for docs.

Nice one, can you please apply your changes to all constructors? Make a commit with a message Closes #47 and it will close this issue.

@FreePhoenix888
Copy link
Member

FreePhoenix888 commented Jul 27, 2021

Should I push it into main branch directly or create a branch and pull request?

@Konard
Copy link
Member

Konard commented Jul 27, 2021

@FreePhoenix888 directly.

@TwinkmrMask
Copy link
Member

TwinkmrMask commented Jul 27, 2021

Gentlemens, the <summary> tag is closed 2 times
There was also an agreement to write at the beginning of <param> Представляет/Represents something
For example:

Represents the minimum value of the range.

@FreePhoenix888
Copy link
Member

Gentlemens, the <summary> tag is closed 2 times
There was also an agreement to write at the beginning of <param> Представляет/Represents something
For example:

Represents the minimum value of the range.

Do you have any thoughts how to improve documentation here?

@FreePhoenix888 FreePhoenix888 linked a pull request Jul 28, 2021 that will close this issue
@FreePhoenix888 FreePhoenix888 self-assigned this Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants