FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

added 'semver' as core type accelerator for S.M.A.SemanticVersion by oising · Pull Request #4142 · PowerShell/PowerShell · GitHub

added 'semver' as core type accelerator for S.M.A.SemanticVersion - #4142

Merged
Dongbo Wang (daxian-dbw) merged 1 commit into
PowerShell:masterfrom
oising:semver-accelerator
Jul 11, 2017
Merged

added 'semver' as core type accelerator for S.M.A.SemanticVersion#4142
Dongbo Wang (daxian-dbw) merged 1 commit into
PowerShell:masterfrom
oising:semver-accelerator

Conversation

Copy link
Copy Markdown
Contributor

As per #3460

Added [semver] as a core (safe to use in constrained language mode) type accelerator for System.Management.Automation.SemanticVersion.

Individual accelerators are not currently being tested, so no unit tests were added or updated.

Copy link
Copy Markdown
Collaborator

Appveyor temporary failed.

Copy link
Copy Markdown
Contributor Author

Yeah, it seems one of the New-TimeSpan tests is failing. It must have already been broken when I merged to rebase.

Copy link
Copy Markdown
Member

Paul Higinbotham (@PaulHigin) currently SemanticVersion is permitted in constrained language, please review if that's OK.

Copy link
Copy Markdown
Contributor

Oisin Grehan (@oising) PowerShell core types are intended to be the absolute minimum needed in a constrained language interactive session. Can you tell us the scenario where this type is needed and why constrained language is required?

Copy link
Copy Markdown
Contributor Author

Paul Higinbotham (@PaulHigin) I guess I included it in core types for the same reasons you guys included [ModuleSpecification]. Given that NuGet packages are a core (no pun intended) building block of CoreFX, and [semver] being a large part of their identity, it seemed like something that may be more useful than not. I know this is all a bit fuzzy, but it's just what made sense to me.

Copy link
Copy Markdown
Member

Paul Higinbotham (@PaulHigin) I think the rationale is that SemanticVersion is replacing System.Version ( the type of $PSVersionTable.PSVersion is SemanticVersion), so I think it makes sense to allow it in constrained language. But we need your expertise to inspect the implementation of SemanticVersion and see if it's safe to be exposed in constrained language.

Oisin Grehan (oising) commented Jun 30, 2017
edited
Loading

Copy link
Copy Markdown
Contributor Author

Dongbo Wang (@daxian-dbw) Yes, also good points. I took a cursory glance at the implementation and didn't see any red flags, but I'm also not privy to your criteria.

Copy link
Copy Markdown
Contributor

Ok, I think this is reasonable. I'll review for security safety. Thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown
Member

Filed #4221 to track adding tests for type accelerators in PowerShell.

Dongbo Wang (daxian-dbw) merged commit d9828fe into PowerShell:master Jul 11, 2017
Oisin Grehan (oising) deleted the semver-accelerator branch August 21, 2018 17:50
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL