Git guidelines and Powderhouse

[KathleenDollard]

We picked the code name Powderhouse because it is a cool place and we thought it hints at what we are doing. The Powderhouse at Powderhouse Square was actually built as a windmill and is a tall, well built stone bulding whose use has evolved over the years. We're evolving System.CommandLine - although a bit inverse of The Powderhouse evolution. We are keeping the purpose and changing the shape.

Powder is black powder, which is a low explosive used in pyrotechnics and other uses such as building roads. We have decided to blow things up a bit. System.CommandLine is a solid and we love it. As covered in #2338 we need to make changes to get the parser ready for incorporation in the .NET libraries and to make features/subsystems easy to extend.

When you are playing with pyrotechnics, you need guardrails. Here, git is an important set of guardrails, allowing us to see history. Maintaining git history means avoiding large red blocks in PRs and tending to small well described commits. We will ask for updates on PRs that don't support our commitment to tracking.

A distinct clean up effort that we will do late in this restructuring effort will be described in a separate issue, so you can check that your annoyances will be fixed prior to GA.

One thing you may notice in the code is that we have removed globbing in the projects that have old code. This lets us temporarily remove code without touching the file.

All code

• Rename files only in collaboration with the core team (via an issue) and use git mv.
• Do not adjust formatting for files from the existing system. In particular, do not convert to file scoped namespaces. In new files, please use file scoped namespaces. If you're editing a file, please just leave the namespace style as it is.
• Limit style fixes to the part of the code you are working on.
• Do not remove code. Comment it out using C style commenting on separate lines. If the code needs further discussion or the comment out is temporary, place the explanatory comment in a TODO above the block - which may contain a large amount of code:

/* Remove this code as unnecessary (we need extra coffee on all days)
if (itsMonday) BrewExtraCoffee();
*/

// TODO: Consider removing this code. Don't we need extra coffee on all days?
/* 
if (itsMonday) BrewExtraCoffee();
*/

Note that the stylistic change of putting conditional comments in braces on new lines is not made.

Special considerations for testing

We want to be able to track all tests. Some will be deleted because they are no longer relevant - for example, our current plan is to have version be an explicitly added feature, so tests that it is implicitly added are no longer correct. However, we want to track the test and plan an explicit pass on tests. Leaving them in place commented out leaves a location for the reason they have been commented out.

• Do not delete tests, comment them out using C style comments as described above.
• Do not rename tests. If the name is wrong, copy the test and explain in the comment for the commented out original.
• Do perform the equivalent test, if you can't copy the test and explain the difference in the commented out original. For example, if the test used the parser, don't replace it with a direct call Subsystem.Execute.
• At least for subsystems, place existing tests into files/classes named as <subsystemName>FunctionalTests. Many of the existing tests are via the parser, which is a functional test in the new design. Also, these tests will remain in place, so any names like legacy would be incorrect.
• At least for subsystems, place unit tests in files/classes named as <subsystemName>Tests.
• If a test is fundamentally changed, such as needing to explicitly add the response file token replacer, mark these tests with a comment. One comment is sufficient for a group of tests if it is the same change. This is very important because we will use these to check for breaking changes. This will almost always be a breaking change, so also add an item to the breaking change issue Breaking changes for Powderhouse #2364. An example of a comment:

// Powderhouse: _Explanation of change_

// Powderhouse: Changed test since then now need to explicitly add the response file handler

[mhutch]

Do not adjust formatting. In particular, do not convert to file scoped namespaces.

However, please use file scoped namespaces in new files that do not contain code copied from existing files.

[KathleenDollard]

Do not adjust formatting. In particular, do not convert to file scoped namespaces.

However, please use file scoped namespaces in new files that do not contain code copied from existing files.

I updated the guideline to reflect your suggestion

[EugeneKrapivin]

Do you have any plans to integrate into the Generic host pattern, i.e. be part of the generic host, and now wrap it into a middleware in the invocation path?

[KathleenDollard]

@EugeneKrapivin We definitely think that is important. It will not be part of the first phase. That phase will rebuild the functionality of the System.CommandLine project only.