Skip to content

Adding hex format for native command exit codes #21067

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

Merged
merged 1 commit into from
Jul 31, 2025

Conversation

sba923
Copy link
Contributor

@sba923 sba923 commented Jan 13, 2024

PR Summary

This implementation adds the hex-format version of the native command exit code message (#19886)

PR Context

PR Checklist

This PR has 6 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +3 -3
Percentile : 2.4%

Total files changed: 2

Change summary by file extension:
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@@ -48,7 +48,7 @@ Describe 'Native command error handling tests' -Tags 'CI' {

$error[0].FullyQualifiedErrorId | Should -BeExactly 'ProgramExitedWithNonZeroCode'
$error[0].TargetObject | Should -BeExactly $exePath
$stderr[1].Exception.Message | Should -BeExactly "Program `"$exeName`" ended with non-zero exit code: 1."
$stderr[1].Exception.Message | Should -BeExactly "Program `"$exeName`" ended with non-zero exit code: 1 (0x00000001)."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not (0x1)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the hex format used is the fixed-sized 0xNNNNNNNN, for readability's sake. Call it a matter of taste, but I find reading values such as 0xFE234 "less pretty/readable/usable" than 0x000FE234.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an FYI while on Windows the rc is a 32-bit integer, on Unix they are just a single byte. I don't have a particular preference either way just wanted to share that in case it helps to make a decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an FYI while on Windows the rc is a 32-bit integer, on Unix they are just a single byte. I don't have a particular preference either way just wanted to share that in case it helps to make a decision.

Are you somehow suggesting that the parenthesized hex value should use 0x{1:X} format on non-Windows platforms and 0x{1:X8} format on Windows?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say about remove leading zeros.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteveL-MSFT shouldn't that be discussed / decided upon by the team? Who is supposed to have the final say on this?

Copy link
Collaborator

@rkeithhill rkeithhill Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Engine WG discussed this today and agree that on Linux & macOS the hexadecimal format should be 0x{1:X2}. We also suggest removing the formatting string from the resx and put it in code. That is change this:

<value>Program "{0}" ended with non-zero exit code: {1} (0x{1:X8}).</value>

to

<value>Program "{0}" ended with non-zero exit code: {1} (0x{2}).</value>

Then in code, do something like this:

                    const string errorId = nameof(CommandBaseStrings.ProgramExitedWithNonZeroCode);
                    const string hexFormatStr = Platform.IsWindows ? "{0:X8}" : "{0:X2}";

                    string errorMsg = StringUtil.Format(
                        CommandBaseStrings.ProgramExitedWithNonZeroCode,
                        NativeCommandName,
                        _nativeProcess.ExitCode,
                        String.Format(CultureInfo.InvariantCulture, hexFormatStr, _nativeProcess.ExitCode));

@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Jan 22, 2024
@rkeithhill rkeithhill self-requested a review February 5, 2024 19:37
@rkeithhill rkeithhill added the WG-Reviewed A Working Group has reviewed this and made a recommendation label Feb 5, 2024

This PR has 10 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +6 -4
Percentile : 4%

Total files changed: 3

Change summary by file extension:
.cs : +3 -1
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Copy link
Contributor Author

@sba923 sba923 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes made and committed to sba923:master. How do I add this to the PR?

This PR has 12 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +7 -5
Percentile : 4.8%

Total files changed: 4

Change summary by file extension:
.csproj : +1 -1
.cs : +3 -1
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

2 similar comments

This PR has 12 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +7 -5
Percentile : 4.8%

Total files changed: 4

Change summary by file extension:
.csproj : +1 -1
.cs : +3 -1
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

This PR has 12 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +7 -5
Percentile : 4.8%

Total files changed: 4

Change summary by file extension:
.csproj : +1 -1
.cs : +3 -1
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@@ -17,7 +17,7 @@
<ItemGroup>
<!-- This section is to force the version of non-direct dependencies -->
<!-- the following package(s) are from https://github.com/dotnet/fxdac -->
<PackageReference Include="System.Data.SqlClient" Version="4.8.5" />
<PackageReference Include="System.Data.SqlClient" Version="4.8.6" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change need to be part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this clearly doesn't, but the CI builds all fail without that change....

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than this issue, the PR LGTM.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will let someone from the PS team chime in on what's going on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.

Will fix the Code Factor issue, BTW.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix the Code Factor issue, BTW.

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will let someone from the PS team chime in on what's going on here.

I've done this in a separate commit, JIC.

If this has to be done elsewhere (which would totally make sense) I'll probably need some help to exclude that commit from the PR, 'cos that's something I've never done.

Copy link
Contributor

@xtqqczze xtqqczze Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was made in #21084.

I would suggest updating the PR with a merge commit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done this in a separate commit, JIC.

If this has to be done elsewhere (which would totally make sense) I'll probably need some help to exclude that commit from the PR, 'cos that's something I've never done.

@sba923 Just revert that specific commit now that it's been in the PR mentioned above by @xtqqczze and ensure that you rebase from the powershell/main branch as that's cleaner than adding merge commits in your PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I have reverted the commit (which results in the CI build to now fail with /home/runner/work/PowerShell/PowerShell/src/Microsoft.PowerShell.SDK/Microsoft.PowerShell.SDK.csproj : error NU1903: Warning As Error: Package 'System.Data.SqlClient' 4.8.5 has a known high severity vulnerability, https://github.com/advisories/GHSA-98g6-xh36-x2p7 [/home/runner/work/PowerShell/PowerShell/src/powershell-unix/powershell-unix.csproj]), but I can't seem to get the rebase to work. Can you please help me out with that (either using the git CLI or VScode)?

This PR has 12 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +7 -5
Percentile : 4.8%

Total files changed: 4

Change summary by file extension:
.csproj : +1 -1
.cs : +3 -1
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@sba923
Copy link
Contributor Author

sba923 commented Feb 8, 2024

The Windows CI fails, I don't understand why....

image

@iSazonov iSazonov closed this Feb 9, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Feb 9, 2024
@iSazonov
Copy link
Collaborator

iSazonov commented Feb 9, 2024

Reopen to restart CIs.

@iSazonov iSazonov reopened this Feb 9, 2024

This PR has 12 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +7 -5
Percentile : 4.8%

Total files changed: 4

Change summary by file extension:
.csproj : +1 -1
.cs : +3 -1
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@@ -969,11 +969,13 @@ internal override void Complete()
}

const string errorId = nameof(CommandBaseStrings.ProgramExitedWithNonZeroCode);
string hexFormatStr = Platform.IsWindows ? "0x{0:X8}" : "0x{0:X2}";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer #if UNIX over runtime check.

Copy link
Contributor Author

@sba923 sba923 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer #if UNIX over runtime check.

Change made.

Note: the codebase contains a significant number of runtime tests e.g. if (Platform.IsWindows)...

This PR has 20 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +13 -7
Percentile : 8%

Total files changed: 4

Change summary by file extension:
.csproj : +1 -1
.cs : +9 -3
.resx : +1 -1
.ps1 : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@sba923
Copy link
Contributor Author

sba923 commented Feb 9, 2024

This time the MacOS CI fails...

image

image

@xtqqczze
Copy link
Contributor

We prefer #if UNIX over runtime check.

Change made.

Note: the codebase contains a significant number of runtime tests e.g. if (Platform.IsWindows)...

@iSazonov FYI, since #16760 the JIT will turn Platform.IsWindows into a constant, e.g. sharplab.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Feb 22, 2024
@sba923
Copy link
Contributor Author

sba923 commented Feb 22, 2024

This time the MacOS CI fails...

image

image

what can I do about this?

@sba923
Copy link
Contributor Author

sba923 commented Mar 29, 2024

What's still blocking this PR?

@kilasuit
Copy link
Collaborator

@sba923 this will get another review before being accepted - also please see the comment I made above about reverting that commit & rebasing from main

@sba923
Copy link
Contributor Author

sba923 commented May 25, 2024

What's still blocking this PR? I'll probably need help with that rebasing action...

@xtqqczze
Copy link
Contributor

What's still blocking this PR? I'll probably need help with that rebasing action...

git fetch upstream && git reset --soft 6df380910 && git commit --amend -C HEAD && git rebase upstream/master && git push --force-with-lease

@sba923
Copy link
Contributor Author

sba923 commented Jun 8, 2024

Last CI run reports PowerShell-Windows-Packaging-CI — This check was skipped.

What should I do next?

@sba923
Copy link
Contributor Author

sba923 commented Jun 19, 2024

@daxian-dbw what's still blocking this PR? Is there anything I can / should do?

@xtqqczze
Copy link
Contributor

@daxian-dbw @TravisEz13 @adityapatwardhan @anmenaga
All checks have passed, could you please review?

@sba923
Copy link
Contributor Author

sba923 commented Jul 7, 2024

@daxian-dbw @TravisEz13 @adityapatwardhan @anmenaga All checks have passed, could you please review?

🏓

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Jul 15, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Jul 23, 2024
@sba923
Copy link
Contributor Author

sba923 commented Jul 23, 2024

If I understand correctly, this is blocked because it hasn't been reviewed....

@rkeithhill
Copy link
Collaborator

@daxian-dbw Any chance you could review this? It's a pretty small & straightforward PR.

@sba923
Copy link
Contributor Author

sba923 commented Jun 18, 2025

@rkeithhill / @daxian-dbw is there anything I can / should do to unblock this?

@daxian-dbw daxian-dbw closed this Jul 29, 2025
@daxian-dbw daxian-dbw reopened this Jul 29, 2025
@daxian-dbw daxian-dbw added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed Review - Needed The PR is being reviewed labels Jul 29, 2025
@iSazonov
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@iSazonov iSazonov changed the title Adding hex format for native command exit codes (#19886) Adding hex format for native command exit codes Jul 31, 2025
@iSazonov iSazonov merged commit a993992 into PowerShell:master Jul 31, 2025
39 of 42 checks passed
Copy link
Contributor

microsoft-github-policy-service bot commented Jul 31, 2025

📣 Hey @@sba923, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Engine Indicates that a PR should be marked as an engine change in the Change Log Extra Small WG-Reviewed A Working Group has reviewed this and made a recommendation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants