GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Cleanup Format-Hex by vexx32 · Pull Request #8683 · PowerShell/PowerShell

Conversation

@vexx32

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) commented Jan 18, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

PR Summary

Cleanup for code style prior to #8674

/cc Ilya (@iSazonov) ❤️

PR Context

Format-Hex can use a little love in the style department. Added some XML comments that were missing, tidied up existing ones, and made sure the code style overall is consistent with how we've been doing in recent PRs according to StyleCop and other linters.

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • Documentation needed
      • Issue filed:
  • Testing - New and feature
    • N/A or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
      • Add [feature] to your commit messages if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vexx32
📝 ♻️ Clean up Format-Hex
9c7677b
@vexx32
Rain Sallow (/u/ta11ow) (vexx32) requested review from James Truher (JamesWTruher) and Paul Higinbotham (PaulHigin) as code owners January 18, 2019 14:43
iSazonov
Ilya (iSazonov) reviewed Jan 18, 2019
View reviewed changes
Comment thread ...rosoft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/format-hex/Format-Hex.cs Outdated
}
}
catch (IOException ioException)
catch (IOException fileException)

@iSazonov Ilya (iSazonov) Jan 18, 2019

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It is not always true :-) Please revert.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) Jan 18, 2019

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

CodeFactor complained that the io prefix looks too much like a Hungarian prefix, from memory. Is this simply not a concern?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Jan 18, 2019

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We frequently use catch (IOException e).
(I guess we will disable the check.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vexx32 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@iSazonov
Ilya (iSazonov) requested a review from Steve Lee (SteveL-MSFT) January 18, 2019 15:08
@iSazonov Ilya (iSazonov) added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Jan 18, 2019
@iSazonov Ilya (iSazonov) self-assigned this Jan 18, 2019
@vexx32
📝 Address PR comments
363548f
iSazonov
Ilya (iSazonov) approved these changes Jan 18, 2019
View reviewed changes
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Jan 18, 2019
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thanks Rain Sallow (/u/ta11ow) (@vexx32) !

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vexx32 reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@iSazonov
Ilya (iSazonov) merged commit 631833c into PowerShell:master Jan 18, 2019
@iSazonov Ilya (iSazonov) changed the title Cleanup: Format-Hex Cleanup Format-Hex Jan 18, 2019
@vexx32
Rain Sallow (/u/ta11ow) (vexx32) deleted the Style/FormatHex branch January 22, 2019 13:18
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Jan 28, 2019
Format-Hex: Improve mixed-collection piped input and piped streams of input #8674
Merged
8 of 11 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@vexx32 @iSazonov
Cleanup Format-Hex (PowerShell#8683)
dc72c72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@iSazonov Ilya (iSazonov) iSazonov approved these changes
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@PaulHigin Paul Higinbotham (PaulHigin) Awaiting requested review from PaulHigin
@SteveL-MSFT Steve Lee (SteveL-MSFT) Awaiting requested review from SteveL-MSFT

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@iSazonov Ilya (iSazonov)

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@vexx32 @daxian-dbw @iSazonov
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.