starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-09-25 08:24:54 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Guidance: Contributing.md & Coding Style docs needed #1210

New Issue
Closed
opened 2026-01-30 22:19:13 +00:00 by claunia · 11 comments
claunia commented 2026-01-30 22:19:13 +00:00
Owner
Copy Link

Originally created by @dlong11 on GitHub (May 18, 2019).

It would be great if the Windows Terminal Team would provide a contributing.md (Contributing Guide) that would explain the Contribution Workflow, "help wanted", Do's and Don'ts, Commit Messages, etc.

Also adding an official coding style guideline md file would be great. Currently, the CppCoreGuide lines are referenced, but that doesn't give enough guidance around things like

Naming

  • general naming conventions
  • File names
  • Type names
  • Variable names
  • Class names
  • Struct names
  • Constant names
  • Function names
  • Enum names
  • Macro names

Formatting (hopefully clang format gets added and this becomes a non-issue)

  • Line length
  • Spaces vs Tabs
  • braces
  • file types (utf-8, utf-8 BOM, etc...)
  • etc...

maintainer note: we did add clang-format so this kinda is a non-issue

C++ features

  • use of const
  • use of constexpr
  • exceptions
  • friends
  • noexcept
  • casting
  • RTTI
  • auto

Comments

  • Comment style
  • File comments
  • Legal / Copyright
  • Class / Function comments
  • TODO comments
  • etc...

The goal of the style guide should be to describe in detail the dos and don'ts of writing C++ code. (ie -what the Windows Terminal team expects when PRs are submitted) Some of the above are covered in the Cpp Core Guidelines but it would be great to have an official Windows Terminal Guide that explicitly states what the expectations are. The current code doesn't always follow the Cpp Core Guidelines and it is hard to tell if that was on purpose or was just missed when the developer implemented the code.

Good examples of Contributing.md files that could be used as examples / ideas
.Net Core
Opensource guide
React

See also

Originally created by @dlong11 on GitHub (May 18, 2019). <!-- What needs to change? Who is responsible for it? Why is it an open question? --> It would be great if the Windows Terminal Team would provide a contributing.md (Contributing Guide) that would explain the Contribution Workflow, "help wanted", Do's and Don'ts, Commit Messages, etc. Also adding an _official_ coding style guideline md file would be great. Currently, the CppCoreGuide lines are referenced, but that doesn't give enough guidance around things like ### Naming - general naming conventions - File names - Type names - Variable names - Class names - Struct names - Constant names - Function names - Enum names - Macro names ### Formatting (hopefully clang format gets added and this becomes a non-issue) - Line length - Spaces vs Tabs - braces - file types (utf-8, utf-8 BOM, etc...) - etc... _maintainer note: we did add clang-format so this kinda is a non-issue_ ### C++ features - use of const - use of constexpr - exceptions - friends - noexcept - casting - RTTI - auto ### Comments - Comment style - File comments - Legal / Copyright - Class / Function comments - TODO comments - etc... The goal of the style guide should be to describe in detail the dos and don'ts of writing C++ code. (ie -what the Windows Terminal team expects when PRs are submitted) Some of the above are covered in the Cpp Core Guidelines but it would be great to have an official Windows Terminal Guide that explicitly states what the expectations are. The current code doesn't always follow the Cpp Core Guidelines and it is hard to tell if that was on purpose or was just missed when the developer implemented the code. Good examples of Contributing.md files that could be used as examples / ideas [.Net Core](https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/contributing.md) [Opensource guide](https://github.com/github/opensource.guide/blob/master/CONTRIBUTING.md) [React](https://reactjs.org/docs/how-to-contribute.html) --- See also * [ ] #1014 * [ ] #962 * [ ] #887 * [ ] #921 * [ ] #1159
claunia added the Needs-Tag-FixIssue-DocsProduct-MetaArea-BuildArea-CodeHealth labels 2026-01-30 22:19:13 +00:00
claunia commented 2026-01-30 22:19:14 +00:00
Author
Owner
Copy Link

@bitcrazed commented on GitHub (May 23, 2019):

Working on contributing guide here: https://github.com/microsoft/terminal/pull/967

@bitcrazed commented on GitHub (May 23, 2019): Working on contributing guide here: https://github.com/microsoft/terminal/pull/967
claunia commented 2026-01-30 22:19:14 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Jun 3, 2019):

As mentioned here, (there's a 90% chance that link breaks), when we have doc comments, we should always include the "Arguments" and the "Return Value" sections, even if they're void. I believe this will lead to further clarity. It makes it clear we're expecting a function to not have params, so if we ever change the signature, it's obvious the doc comment is out of date.

@zadjii-msft commented on GitHub (Jun 3, 2019): As mentioned [here](https://github.com/microsoft/terminal/pull/825#discussion_r289529114), (there's a 90% chance that link breaks), when we have doc comments, we should always include the "Arguments" and the "Return Value" sections, even if they're void. I believe this will lead to further clarity. It makes it clear we're expecting a function to not have params, so if we ever change the signature, it's obvious the doc comment is out of date.
claunia commented 2026-01-30 22:19:14 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Jun 3, 2019):

[vehement disagreement intensifies]

@DHowett-MSFT commented on GitHub (Jun 3, 2019): `[vehement disagreement intensifies]`
claunia commented 2026-01-30 22:19:14 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Jun 3, 2019):

@DHowett-MSFT @ me bro. Pros/cons?

@zadjii-msft commented on GitHub (Jun 3, 2019): @DHowett-MSFT @ me bro. Pros/cons?
claunia commented 2026-01-30 22:19:14 +00:00
Author
Owner
Copy Link

@dlong11 commented on GitHub (Jun 4, 2019):

Ah - better to resolve it now versus dealing with it hundreds of times in reviews. Same rules == more efficient reviews. 😄

@dlong11 commented on GitHub (Jun 4, 2019): Ah - better to resolve it now versus dealing with it hundreds of times in reviews. Same rules == more efficient reviews. 😄
claunia commented 2026-01-30 22:19:15 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Jun 4, 2019):

I'm mostly concerned that the value it adds is outweighed by the noise it adds. It feels like overdocumenting to tell somebody something they could have reasonably guessed by reading the code... like having a variable called indexIntoColorArray and commenting it with // this is the index into the color array... to which I'd respond "tell me something I couldn't have figured out simply by reading it."

I'm not overly concerned with our code size, but we'd be introducing four lines and a bunch of mental overhead for something that requires 0 lines and 0 mental overhead. Having a field that says "there are no arguments to this function" holds up equally well in the face of somebody changing the function's signature to having no field that ways anything about arguments.

Now, return value is an interesting one. I've had a look over the code, and we have a lot of them that say <none>. We also have some that specify what, if anything, the function throws.

Should that be a separate optional section? Is that too much work to keep in sync? How would we handle things that call things that can throw? It's nearly boundless...

@DHowett-MSFT commented on GitHub (Jun 4, 2019): I'm mostly concerned that the value it adds is outweighed by the noise it adds. It feels like overdocumenting to tell somebody something they could have reasonably guessed by reading the code... like having a variable called `indexIntoColorArray` and commenting it with `// this is the index into the color array`... to which I'd respond "tell me something I couldn't have figured out simply by reading it." I'm not overly concerned with our code size, but we'd be introducing four lines and a bunch of mental overhead for something that requires 0 lines and 0 mental overhead. Having a field that says "there are no arguments to this function" holds up equally well in the face of somebody changing the function's signature to having no field that ways anything about arguments. Now, return value is an interesting one. I've had a look over the code, and we have a _lot_ of them that say `<none>`. We also have some that specify _what, if anything, the function throws_. Should that be a separate optional section? Is that too much work to keep in sync? How would we handle things that call things that can throw? It's nearly boundless...
claunia commented 2026-01-30 22:19:15 +00:00
Author
Owner
Copy Link

@dlong11 commented on GitHub (Jun 4, 2019):

I have held back my opinion on this subject because it appeared the documentation was a hard requirement. You know when in Rome...

Since you brought this up though 😄 - I have a couple of observations. Most of the function comments in this repo are in the cpp files and they are very verbose. (line wise) Which interferes with the readability of the code. Also, most can be derived from the type and names of the variables. They don't add value IMHO. If comments are a requirement, can they be a simpliefied version that are added to the header file instead of the implementation file?

Simple example - setting a cursor (totally made up example)

Current way
// Routine Description:
// Sets the currently visible cursor. If |cursor| is CursorType::Null, the cursor used
// before the current is restored.
// Arguments:
// - cursor - The Cursor
// Return Value:
// - none
void SetCursor(CursorType::Cursor cursor) 
{
<code>
}

vs. (a simple comment in the header file. The cpp comments would be removed)

// Sets the currently visible cursor. If |cursor| is CursorType::Null, the cursor used
// before the current is restored.
void SetCursor(CursorType::Cursor cursor);

Pros

  • helps the readability of the cpp files (less noise)
  • less maintenance because you are not documenting every param (and prevents all of the nits in Code Reviews 😄 )
  • complex or unordinary params can still be documented

Cons

  • forces the user to look in the header if they have a question about the method
  • sometimes a parameter may not get documented because the implementer thought it was unnecessary. (<- could be caught in a review though)
  • a modified comment will cause more recompiling because of dependencies

Obviously, this is a huge departure from the current code base. I am just throwing this out as an idea. Also the above doesn't mean you can't put a comment in the cpp files function implementation. If the code is "tricky" you could document the operation in the cpp file.

Again - just an idea. I know groups have strong opinions on these types of issues. 😄

I am assuming this style of comments has been used for a while. Do you regularly rely on the comments?

@dlong11 commented on GitHub (Jun 4, 2019): I have held back my opinion on this subject because it appeared the documentation was a hard requirement. You know when in Rome... Since you brought this up though 😄 - I have a couple of observations. Most of the function comments in this repo are in the cpp files and they are very verbose. (line wise) Which interferes with the readability of the code. Also, most can be derived from the type and names of the variables. They don't add value IMHO. If comments are a requirement, can they be a simpliefied version that are added to the header file instead of the implementation file? Simple example - setting a cursor (totally made up example) ``` Current way // Routine Description: // Sets the currently visible cursor. If |cursor| is CursorType::Null, the cursor used // before the current is restored. // Arguments: // - cursor - The Cursor // Return Value: // - none void SetCursor(CursorType::Cursor cursor) { <code> } vs. (a simple comment in the header file. The cpp comments would be removed) // Sets the currently visible cursor. If |cursor| is CursorType::Null, the cursor used // before the current is restored. void SetCursor(CursorType::Cursor cursor); ``` Pros - helps the readability of the cpp files (less noise) - less maintenance because you are not documenting every param (and prevents all of the nits in Code Reviews 😄 ) - complex or unordinary params can still be documented Cons - forces the user to look in the header if they have a question about the method - sometimes a parameter may not get documented because the implementer thought it was unnecessary. (<- could be caught in a review though) - a modified comment will cause more recompiling because of dependencies Obviously, this is a huge departure from the current code base. I am just throwing this out as an idea. Also the above doesn't mean you can't put a comment in the cpp files function implementation. If the code is "tricky" you could document the operation in the cpp file. Again - just an idea. I know groups have strong opinions on these types of issues. 😄 I am assuming this style of comments has been used for a while. Do you regularly rely on the comments?
claunia commented 2026-01-30 22:19:15 +00:00
Author
Owner
Copy Link

@carlos-zamora commented on GitHub (Jun 5, 2019):

Regarding contributing, here's my thoughts. There's two relevant things:

  • assignee
  • "Help-Wanted" tag

Assignee should be the person/people working on the issue that have a good level of knowledge/understanding of it. So, if you've worked on similar things or you know who to talk to on certain issues, you should probably assign yourself. I'd say just have one team member be the point person assigned on there so that we don't have too much assigned to anybody.

Now, the "Help-Wanted" tag is where things get interesting. As a casual contributor (not on the team), I can see that this is a good place to start. Open up any issues with this tag and do the following:

  • if it's a PR, help review it
  • if it's an issue, submit my thoughts as comments
  • if it's a feature request or bug mention,
    • provide more info on the topic (i.e.: corner cases, things to look out for when implementing fix, etc.)
      But if I want to get even MORE involved and want to take a crack at writing the code, here's what I as a casual contributor should do:
  • comment @ the assignee asking if I can do it
  • with approval (really the assignee just acknowledging that they don't have to worry about implementing it anymore) start working on it
  • submit a PR and get my code into the repo!!

tl;dr I think we should use the "Help-Wanted" tag as a way of having the community jump in. Definitely do it with easy-starters. And have the Assignee option serve as a liason/guide.

@carlos-zamora commented on GitHub (Jun 5, 2019): Regarding contributing, here's my thoughts. There's two relevant things: - assignee - "Help-Wanted" tag Assignee should be the person/people working on the issue that have a good level of knowledge/understanding of it. So, if you've worked on similar things or you know who to talk to on certain issues, you should probably assign yourself. I'd say just have one team member be the point person assigned on there so that we don't have too much assigned to anybody. Now, the "Help-Wanted" tag is where things get interesting. As a casual contributor (not on the team), I can see that this is a good place to start. Open up any issues with this tag and do the following: - if it's a PR, help review it - if it's an issue, submit my thoughts as comments - if it's a feature request or bug mention, - provide more info on the topic (i.e.: corner cases, things to look out for when implementing fix, etc.) But if I want to get even MORE involved and want to take a crack at writing the code, here's what I as a casual contributor should do: - comment @ the assignee asking if I can do it - with approval (really the assignee just acknowledging that they don't have to worry about implementing it anymore) start working on it - submit a PR and get my code into the repo!! tl;dr I think we should use the "Help-Wanted" tag as a way of having the community jump in. Definitely do it with easy-starters. And have the Assignee option serve as a liason/guide.
claunia commented 2026-01-30 22:19:15 +00:00
Author
Owner
Copy Link

@dlong11 commented on GitHub (Jun 5, 2019):

Could the repo have a CODE_OWNERS.TXT as the CoreClr has? This might be helpful. I'm not 100% sold on people looking at this, but it is one way for people to know who they should ask.

@dlong11 commented on GitHub (Jun 5, 2019): Could the repo have a [CODE_OWNERS.TXT](https://github.com/dotnet/coreclr/blob/master/CODE_OWNERS.TXT) as the CoreClr has? This might be helpful. I'm not 100% sold on people looking at this, but it is one way for people to know who they should ask.
claunia commented 2026-01-30 22:19:15 +00:00
Author
Owner
Copy Link

@bitcrazed commented on GitHub (Jun 6, 2019):

The new Contributor's guide is now in the docs folder, along with specs folder & spec template.

Coding standards etc. next-up.

@bitcrazed commented on GitHub (Jun 6, 2019): The new [Contributor's guide](https://github.com/microsoft/terminal/blob/master/doc/contributing.md) is now in the docs folder, along with [specs](https://github.com/microsoft/terminal/tree/master/doc/specs) folder & [spec template](https://github.com/microsoft/terminal/blob/master/doc/specs/spec-template.md). Coding standards etc. next-up.
claunia commented 2026-01-30 22:19:15 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Mar 5, 2024):

We never ended up standardizing on these in doc form! Thanks all for playing, however! We shipped at the very least a code formatter, and there's work underway to get clang-tidy going.

@DHowett commented on GitHub (Mar 5, 2024): We never ended up standardizing on these in doc form! Thanks all for playing, however! We shipped at the very least a code formatter, and there's work underway to get clang-tidy going.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Build Area-CodeHealth Issue-Docs Needs-Tag-Fix Product-Meta
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#1210
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?