Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Rename $IsOSX to $IsMacOS#4757

Merged
TravisEz13 merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
SteveL-MSFT:rename-ismacosSteveL-MSFT/PowerShell:rename-ismacosCopy head branch name to clipboard
Sep 7, 2017
Merged

Rename $IsOSX to $IsMacOS#4757
TravisEz13 merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
SteveL-MSFT:rename-ismacosSteveL-MSFT/PowerShell:rename-ismacosCopy head branch name to clipboard

Conversation

@SteveL-MSFT

Copy link
Copy Markdown
Member

PS-Committee decided we should be consistent with our naming and conform to Apple's use of macOS instead of OSX, however, for readability and consistently we are staying with Pascal casing.

Also renamed instances of variations of OSX to macOS for consistency in comments/text (separate commit to make it easier to review)

Fix #4700

@daxian-dbw

Copy link
Copy Markdown
Member

I don't see changes to build.psm1, is that file missed from this PR?

@SteveL-MSFT

SteveL-MSFT commented Sep 6, 2017

Copy link
Copy Markdown
Member Author

@daxian-dbw ok, looks like VSCode cached exclusion list and I previously excluded build.psm1, will fix. looks like .md files were excluded as well.

@iSazonov

iSazonov commented Sep 6, 2017

Copy link
Copy Markdown
Collaborator

Apple Inc. uses macOS not MacOS - maybe we should use the same too? - $IsmacOS

@SteveL-MSFT

Copy link
Copy Markdown
Member Author

@iSazonov we had that discussion in the issue, the consensus is that $IsMacOS is preferred over $IsmacOS for readability and other languages (like Java) use isMacOS, so it's already somewhat common.

@iSazonov iSazonov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment thread tools/travis.ps1 Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor comment - maybe replace "OSX" too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The file stored in Azure for the badge has OSX in the filename. I'm fine leaving that one.

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The package name for macOS should also be changed, but that can be a separate PR.

@markekraus

Copy link
Copy Markdown
Contributor

Should this run through the CI with [feature] to ensure none of the feature tests broke with the change?

@SteveL-MSFT

Copy link
Copy Markdown
Member Author

@markekraus good point, I'll resubmit

@SteveL-MSFT

Copy link
Copy Markdown
Member Author

AppVeyor failure is due to the current issue causing nightly to fail and @dantraMSFT is looking into it

@dantraMSFT

Copy link
Copy Markdown
Contributor

Yes, the failure is due to #4763. You can ignore it.

@SteveL-MSFT

Copy link
Copy Markdown
Member Author

Rebased to pick up fix

@TravisEz13 TravisEz13 merged commit 7c9b188 into PowerShell:master Sep 7, 2017
@mirichmo mirichmo added the Breaking-Change breaking change that may affect users label Sep 12, 2017
@SteveL-MSFT SteveL-MSFT deleted the rename-ismacos branch October 16, 2017 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking-Change breaking change that may affect users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.