Styleguide First Pass #68

JustinWinningham · 2020-04-22T01:18:23Z

First pass at styleguide for C# documentation

strlngd · 2020-04-22T02:42:39Z

Very cool! The documentation definitely needs a more consistent format. I would also like to contribute towards this when I have the time (~2 days).

A few things to note:

Please convert all instances of <span style= "font-size:12px"> encasing the tables with a div. Using span in this specific way will result in a compilation error.
I have hosted this on the website here. Hopefully this will help with formatting and styling the above guide.

Anakael · 2020-04-22T21:06:55Z

STYLEGUIDE.md

+|
+</span>
+
+> Omit elements entirely if not relavent to that class (eg: parent class)


Why that? Parent class and interfaces - usefull information.

Anakael · 2020-04-22T21:10:25Z

STYLEGUIDE.md

+* Type (with link to that object or objects)
+* Notes
+
+## Variables


Where to describe consts?
in this section, or another?

Fixed some typos, cleaned up the tables, and changed spans to divs per feedback request. Removed some overly strict requirements too.

Adjusted Class variables table to add row for const / read only / static / etc

Fixed broken Method Table

More Fixes

Last set of fixes, removed extra table columns and more cleanup

JustinWinningham · 2020-04-22T22:01:06Z

Ready for Re-review :)

Anakael · 2020-04-22T22:50:30Z

STYLEGUIDE.md

+<span style= "font-size:12px">
+
+Using / Namespaces | | <span style='color:yellow'>Taleworlds.Core</span>| | | | <span style='color:green'>Saveable</span> |
+:---|---|---|---|---|---|---:|
+[System](EXAMPLE) | | | | | | 10000 |
+[System.Xml](EXAMPLE)|
+[TaleWorlds.SaveSystem](EXAMPLE) |
+|
+</span>


Is that table correct?
It render as:

But it make no sense..

The left column shows all the includes, the yellow is the specific namespace, and savable is green, the savable class number is 10000. I'm more than open to better recommendations

Table should contain similar data. But without table it will be over wide.
So, maybe table is the best now.

Anakael · 2020-04-22T22:59:15Z

STYLEGUIDE.md

+
+# Enums
+
+Enums should be placed in their own file at the Class level (on their own page under a package) with the following format:


Where to describe enums, which defined in class? In separate file? In that case info about using is redundant.

I was basing off of the decompiled dlls. Enums are defined in their and don't have anything else in them afaik:

public enum WeaponClass
{
// Token: 0x040005BF RID: 1471
Undefined,
// Token: 0x040005C0 RID: 1472
Dagger,
...

I am working on Hero class. And it define structs and enums in self.
Where would be good place to document them?

If they are unique / private to the class they should probably go somewhere within the class, if they live outside the class then they should go as their own page in the Enums section. Open to recommendations for the former.

Ok. I will place it to class on my case.
But according to styleguides enum should be separated from class.
So that lack on develop side :D (also with public variables)

Anakael · 2020-04-22T23:00:25Z

STYLEGUIDE.md

+|
+</span>
+
+### Values


Where H2 header?

fixed to H2

Anakael · 2020-04-22T23:04:59Z

STYLEGUIDE.md

+
+Unlike the root and package level READMEs, the class README will contain more than just links to child folders. Each class will start with a **Class Overview Panel**:
+
+### [Parent Object Name](EXAMPLE)


File should start with H1 header.
I suggest move info about parent and interfaces to H2 after description.

I'd argue its good to have quick access to the parrent class right next to the classname, but thats just me.

Yeah, it's good. So that's why I suggested h2 after description.

But when you open a class readme - you, most likely want to read about this certain class first, not related.

Anakael · 2020-04-22T23:07:48Z

STYLEGUIDE.md

+|...|...|...|...|
+</div>
+
+Methods should follow parameters, seperated by a H1 header.


-> H1
Is typo here?

fixed to be H2. Poor wording on my part. The idea is to have a clear break between each section

Anakael · 2020-04-22T23:13:47Z

STYLEGUIDE.md

+
+<div style= "font-size:12px">
+
+| Type | Method Name | Number of Params |


Number of Params - redundant?

Params better, I think.

Agreed. Overthought it while writing. Fixed.

Anakael · 2020-04-22T23:19:25Z

STYLEGUIDE.md

+### [Parent Object Name](EXAMPLE)
+## Method Name
+
+| Parameters | Type | default | Optional? |


MnB2 use C#, not TS. Type -> Parameters is better :)
Methods discription place in separate file, do I understand correctly?
In that case we don't need table here, because we have almost the same in another place. In addition, discription would be better placed here, not in another header.
Example: https://docs.python.org/3/library/math.html

Also, Optional and default have the same meaning.

oof. Not a C# master by any means, Fixed.

Each file (Classes, Methods, etc) will focus on itself. So the Methods will have their description in their own file, not in their class file. I agree that this table could be optional, but users may navigate straight to a method from very far away (so to speak) in the documentation and won't arrive by going directly through their parrent class (or struct, w/e) tree. If you think this is still worth removing after that consideration, I'll remove.

To summarize, I suggest:
Put methods declaration in class (when I read about class, I want to see it interface) as list (not table, because table assume that datas similar by size)
Put methods definition in separate file (without table).
In that case class won't be overwhelmed and definition won't be stretched out.

Lots of feedback fixes from Anakael's feedback

strlngd · 2020-04-23T21:00:59Z

For this Style Guide, are you proposing that the C# API should include the namespace in each path? In other words, something like:

C# API > Library > Namespace > Class

Right now it is organized like this:

C# API > Library > Class

My vote is to stick with the current setup and then include the namespace at the top of the documentation for each class. Let me know your thoughts.

Also, I would rename instances of Package to Library/Namespace depending on the context.

JustinWinningham · 2020-04-24T01:29:41Z

C# API > Library (by dll package) > Class > Method As for renaming package to library / namespace, im fine with either.

Anakael · 2020-04-26T21:13:55Z

STYLEGUIDE.md

+
+The above would be a valid snippet for the ```InformationMessage``` Class, but can be longer if the extra length is used to give important context about the snippet.
+
+Directly after this intro snippet, we have a section outlining the variables of the class. All variables are assumed to have private setters unless otherwise noted. Each variable should contain all of the following information:


If it private - why it is required to be mentioned?

Anakael · 2020-04-26T21:14:05Z

STYLEGUIDE.md

+The above would be a valid snippet for the ```InformationMessage``` Class, but can be longer if the extra length is used to give important context about the snippet.
+
+Directly after this intro snippet, we have a section outlining the variables of the class. All variables are assumed to have private setters unless otherwise noted. Each variable should contain all of the following information:
+


Styleguide First Pass

729a863

First pass at styleguide for C# documentation

strlngd self-assigned this Apr 22, 2020

strlngd linked an issue Apr 22, 2020 that may be closed by this pull request

Styles in book #58

Closed

Anakael suggested changes Apr 22, 2020

View reviewed changes

daxter154 added 5 commits April 22, 2020 14:47

Typo and PR Feedback Fixes

6bdac1f

Fixed some typos, cleaned up the tables, and changed spans to divs per feedback request. Removed some overly strict requirements too.

More PR Feedback fixes

dc9e8fd

Adjusted Class variables table to add row for const / read only / static / etc

Still more typo fixes

5c62920

Fixed broken Method Table

Still more fixes

742923e

More Fixes

Last set of fixes for now

02d11f8

Last set of fixes, removed extra table columns and more cleanup

Anakael suggested changes Apr 22, 2020

View reviewed changes

Anakael reviewed Apr 22, 2020

View reviewed changes

More feedback Fixes - still WIP

ad0d63a

Lots of feedback fixes from Anakael's feedback

strlngd removed a link to an issue Apr 23, 2020

Styles in book #58

Closed

strlngd linked an issue Apr 23, 2020 that may be closed by this pull request

Documentation - Style Guide #67

Open

Anakael reviewed Apr 26, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Styleguide First Pass #68

Styleguide First Pass #68

JustinWinningham commented Apr 22, 2020

strlngd commented Apr 22, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 22, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 22, 2020

JustinWinningham commented Apr 22, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 23, 2020

Anakael Apr 23, 2020

Anakael Apr 22, 2020 •

edited

Loading

JustinWinningham Apr 23, 2020

Anakael Apr 23, 2020

JustinWinningham Apr 23, 2020

Anakael Apr 23, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 23, 2020

Anakael Apr 22, 2020 •

edited

Loading

JustinWinningham Apr 23, 2020

Anakael Apr 23, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 23, 2020

Anakael Apr 22, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 23, 2020

Anakael Apr 22, 2020

Anakael Apr 22, 2020

JustinWinningham Apr 23, 2020

JustinWinningham Apr 23, 2020

Anakael Apr 23, 2020

strlngd commented Apr 23, 2020

JustinWinningham commented Apr 24, 2020

Anakael Apr 26, 2020

Anakael Apr 26, 2020


		# Enums

		Enums should be placed in their own file at the Class level (on their own page under a package) with the following format:


		Unlike the root and package level READMEs, the class README will contain more than just links to child folders. Each class will start with a Class Overview Panel:

		### [Parent Object Name](EXAMPLE)


		<div style= "font-size:12px">

		\| Type \| Method Name \| Number of Params \|


		The above would be a valid snippet for the ```InformationMessage``` Class, but can be longer if the extra length is used to give important context about the snippet.

		Directly after this intro snippet, we have a section outlining the variables of the class. All variables are assumed to have private setters unless otherwise noted. Each variable should contain all of the following information:

Styleguide First Pass #68

Are you sure you want to change the base?

Styleguide First Pass #68

Conversation

JustinWinningham commented Apr 22, 2020

strlngd commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JustinWinningham commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anakael Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anakael Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strlngd commented Apr 23, 2020

JustinWinningham commented Apr 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anakael Apr 22, 2020 •

edited

Loading

Anakael Apr 22, 2020 •

edited

Loading