From 7b3ace4be41683e47b22a3020f87a2923763e18f Mon Sep 17 00:00:00 2001 From: "Jason Shirk (POWERSHELL)" Date: Thu, 21 Jul 2016 15:50:04 -0700 Subject: [PATCH 1/3] Use one fragment per line in Markdown --- docs/building/internals.md | 2 +- docs/building/osx.md | 36 ++++++++++++++++------------------- docs/building/windows-core.md | 9 ++++----- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/docs/building/internals.md b/docs/building/internals.md index f9bc607295..4a4e11af96 100644 --- a/docs/building/internals.md +++ b/docs/building/internals.md @@ -5,7 +5,7 @@ The purpose of this document is to explain build process **internals** with subt This document is not by any means complete. The ultimate source of truth is the code in `.\build.psm1` that's getting executed on the corresponding CI system. -This document assumes, that you can successfully build PowerShell from sources for your platform. +This document assumes that you can successfully build PowerShell from sources for your platform. Top directory diff --git a/docs/building/osx.md b/docs/building/osx.md index 8ed403dbe5..50f77dc70f 100644 --- a/docs/building/osx.md +++ b/docs/building/osx.md @@ -12,10 +12,10 @@ CoreFX issue #[7731][]. Environment =========== -You will want [Homebrew](http://brew.sh/), the missing package manager -for OS X. Once installed, follow the same instructions to download and -install a self-hosted copy of PowerShell on your OS X machine, and use -`Start-PSBootstrap` to install the dependencies. +You will want [Homebrew](http://brew.sh/), the missing package manager for OS X. +Once installed, follow the same instructions to download and +install a self-hosted copy of PowerShell on your OS X machine, +and use`Start-PSBootstrap` to install the dependencies. The `Start-PSBootstrap` function does the following: @@ -24,15 +24,15 @@ The `Start-PSBootstrap` function does the following: - Uninstalls any prior versions of .NET CLI - Downloads and installs the latest .NET CLI 1.0.0-preview2 SDK to `~/.dotnet` -If you want to use `dotnet` outside of `Start-PSBuild`, add `~/.dotnet` to your -`PATH` environment variable. +If you want to use `dotnet` outside of `Start-PSBuild`, +add `~/.dotnet` to your `PATH` environment variable. error: Too many open files -------------------------- -Due to a [bug][809] in NuGet, the `dotnet restore` command will fail -without the limit increased. Run `ulimit -n 2048` to fix this in your -session; add it your shell's profile to fix it permanently. +Due to a [bug][809] in NuGet, the `dotnet restore` command will fail without the limit increased. +Run `ulimit -n 2048` to fix this in your session; +add it your shell's profile to fix it permanently. We cannot do this for you in in the build module due to #[847][]. @@ -71,15 +71,11 @@ brew link --force openssl Build using our module ====================== -Instead of installing the Ubuntu package of PowerShell, download the -`pkg` from our GitHub releases page using your browser, complete the -wizard, start a `powershell` session, and use `Start-PSBuild` from the -module. +Instead of installing the Ubuntu package of PowerShell, +download the `pkg` from our GitHub releases page using your browser, complete the wizard, +start a `powershell` session, and use `Start-PSBuild` from the module. -The output directory will be slightly different because your runtime -identifier is different. PowerShell will be at -`./src/powershell/bin/Linux/netcoreapp1.0/osx.10.11-x64/powershell`, -or `osx.10.10` depending on your operating system version. Note that -configration is still `Linux` because it would be silly to make yet -another separate configuration when it's used soley to work-around a -CLI issue. +The output directory will be slightly different because your runtime identifier is different. +PowerShell will be at `./src/powershell/bin/Linux/netcoreapp1.0/osx.10.11-x64/powershell`, +or `osx.10.10` depending on your operating system version. +Note that configration is still `Linux` because it would be silly to make yet another separate configuration when it's used soley to work-around a CLI issue. diff --git a/docs/building/windows-core.md b/docs/building/windows-core.md index 65851e5785..d98027bf71 100644 --- a/docs/building/windows-core.md +++ b/docs/building/windows-core.md @@ -24,9 +24,8 @@ repository and `cd`ed into it. .NET CLI -------- -We use the [.NET Command Line Interface][dotnet-cli] (`dotnet`) to -build PowerShell. The `Start-PSBootstrap` function will automatically -install it and add it to your path: +We use the [.NET Command Line Interface][dotnet-cli] (`dotnet`) to build PowerShell. +The `Start-PSBootstrap` function will automatically install it and add it to your path: ```powershell Import-Module ./build.psm1 @@ -40,8 +39,8 @@ Invoke-WebRequest -Uri https://raw.githubusercontent.com/dotnet/cli/rel/1.0.0/sc ./install.ps1 ``` -If you have any problems installing `dotnet`, please see their -[documentation][cli-docs]. +If you have any problems installing `dotnet`, +please see their [documentation][cli-docs]. If you are using Windows 7, Windows Server 2008 or Windows Server 2012 you will also need to install From 8fcd8065be865ae8db20da5977df9117747ec448 Mon Sep 17 00:00:00 2001 From: "Jason Shirk (POWERSHELL)" Date: Thu, 21 Jul 2016 23:12:46 -0700 Subject: [PATCH 2/3] Coding guidelines first cut --- docs/dev-process/coding-guidelines.md | 88 ++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 8 deletions(-) diff --git a/docs/dev-process/coding-guidelines.md b/docs/dev-process/coding-guidelines.md index 110d38457d..da26e8012e 100644 --- a/docs/dev-process/coding-guidelines.md +++ b/docs/dev-process/coding-guidelines.md @@ -1,9 +1,81 @@ -author: Jason -> file encodings! -> performance standards -> platform support -> how to write portable code, tests, UI across platforms -> #ifdef -> note about internal APIs? -> avoid take dependency on code via reflection +# C# Coding Style + +## Coding Conventions + +As a general rule, our coding convention is to follow the style of the surrounding code. +Avoid reformatting any code when submitting a PR as it obscures the functional changes of your change. +We run the [.NET code formatter tool](https://github.com/dotnet/codeformatter) regularly help keep consistent formatting. + +A basic rule of formatting is to use "Visual Studio defaults". +Here are some general guidelines + +* No tabs, indent 4 spaces. +* Braces usually go on their own line, + with the exception of single line statements that are properly indented. +* Use `_camelCase` for instance fields, + use `readonly` where possible. +* Use of `this` is neither encouraged nor discouraged. +* Avoid more than one blank empty line. +* Public members should use [doc comments](https://msdn.microsoft.com/en-us/library/b2s063f7.aspx), + internal members may use doc comments but it is not enouraged. +* Public members in a namespace that ends with `Internal`, + for example `System.Management.Automation.Internal` are not considered a supported public API. + Such members are necessarily public as implementation details in code shared between C# and PowerShell script, + or must be available publically by generated code. +* File encoding should be ASCII (preferred) + or UTF8 (with BOM) if absolutely necessary. + +## Preprocessor defines + +There are 3 primary preprocessor macros we define during builds: + +* DEBUG - guard code that should not be included in release builds +* CORECLR - guard code that differs between Full CLR and CoreCLR +* UNIX - guard code that is specific to Unix (Linux and Mac OS X) + +Any other preprocessor defines found in the source are used for one-off custom builds, +typically to help debug specific scenarios. + +### Runtimes + +The PowerShell repo is used to build PowerShell targeting CoreCLR as well as CLR 4.5. + +Code under !CORECLR must build against CLR 4.5. +We will not accept changes that require a later version of the full CLR. +In extremely rare cases, we may use reflection to use an API in a later version of the CLR, +but the feature must robustly handle running with CLR 4.5. + +We may reject code under !CORECLR without explanation because +we do not support installation or testing of such code in this repo. +All new features should support CoreCLR. + +## Performance considerations + +PowerShell has a lot of performance sensitive code as well as a lot of inefficient code. +We have some guidelines that we typically apply widely even in less important code +because code and patterns are copied we want certain inefficient code to stay out of the performance critical code. + +Some general guidelines: + +* Avoid LINQ - it can create lots of avoidable garbage +* Prefer `for` and `foreach`, + with a slight preference towards `for` when you're uncertain if `foreach` allocates an iterator. +* Avoid `params` arrays, prefer adding overloads with 1, 2, 3, and maybe more parameters. +* Be aware of APIs such as `String.Split(params char[])` that do not provide overloads to avoid array allocation. + When calling such APIs, reuse a static array when possible. +* Avoid unnecessary memory allocation in a loop. + Move the memory allocation outside the loop if possible. + +## Portable code + +The PowerShell code base started on Windows and depends on many Win32 APIs through P/Invoke. +Going forward, we try to depend on CoreCLR to handle platform differences, +so avoid adding new P/Invoke calls where a suitable alternative exists in .NET. + +Try to minimize the use of `#if UNIX` and `#if CORECLR`. +When absolutely necessary, avoid duplicating more code than necessary, +and instead prefer introducing helper functions to minimize the platform differences. + +When adding platform dependent code, prefer preprocessor directives +over runtime checks. \ No newline at end of file From d4c8688c13fa08726993398db6ed1eaf1898e67c Mon Sep 17 00:00:00 2001 From: Jason Shirk Date: Fri, 22 Jul 2016 08:50:21 -0700 Subject: [PATCH 3/3] Update issue labels doc --- docs/dev-process/issue-label-descriptions.md | 39 ++++++++++++-------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/docs/dev-process/issue-label-descriptions.md b/docs/dev-process/issue-label-descriptions.md index 3b23ae2f42..9339d64285 100644 --- a/docs/dev-process/issue-label-descriptions.md +++ b/docs/dev-process/issue-label-descriptions.md @@ -11,20 +11,21 @@ author: Jason/Andy These labels describe what feature area of PowerShell that an issue addresses. -* `Area-Build`: -* `Area-Cmdlets`: -* `Area-Console`: -* `Area-Debugging`: -* `Area-Documentation`: -* `Area-Engine`: -* `Area-Intellisense`: -* `Area-Language`: -* `Area-OMI`: -* `Area-PackageManagement`: -* `Area-Performance`: -* `Area-Portability`: -* `Area-PSReadline`: -* `Area-Test`: +* `Area-Build`: build issues +* `Area-Cmdlets`: cmdlets in any module +* `Area-Console`: the console experience +* `Area-Debugging`: debugging PowerShell script +* `Area-Documentation`: PowerShell *repo* documentation issues, general PowerShell doc issues go [here](https://github.com/PowerShell/PowerShell-Docs/issues) +* `Area-Engine`: core PowerShell engine, interpreter, runtime +* `Area-Intellisense`: tab completion +* `Area-Language`: parser, language semantics +* `Area-OMI`: omi +* `Area-PackageManagement`: PackageManagement related issues +* `Area-Performance`: a performance issue +* `Area-Portability`: anything affecting script portability +* `Area-PSReadline`: PSReadLine +* `Area-SideBySide`: Side by side support +* `Area-Test`: tests or test infrastructure ### Operating systems @@ -34,6 +35,14 @@ These are for issues that are specific to certain operating systems: * `OS-Windows` ### Process Tags - + +Issues can be in one of the following states: +* `0 - Backlog` : We've acknowledged the issue but have no immediate plans to address it. +* `1 - Planning` : The issue requires some design or discussion before coding can begin. +* `2 - Ready` : Any design or discussion is essentially done, coding has not yet begun though. +* `3 - Working` : The assignee(s) are actively working on the issue. +* `4 - In Review` : The issue is being reviewed. +The assignee(s) are responsible for signing off before the PR will be merged. + * `help wanted` : We are looking for someone to work on this issue. * `need review` : This Pull Request is being reviewed. Please see [Pull Request - Code Review](../../.github/CONTRIBUTING.md#pull-request-code-review)