Skip to content

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

@oising

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.

@iSazonov

Copy link
Copy Markdown
Collaborator

Appveyor temporary failed.

@oising

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.

@daxian-dbw

Copy link
Copy Markdown
Member

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

@PaulHigin

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?

@oising

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.

@daxian-dbw

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.

@oising

Oisin Grehan (oising) commented Jun 30, 2017

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.

@PaulHigin

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.

LGTM

@daxian-dbw

Copy link
Copy Markdown
Member

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

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit d9828fe into PowerShell:master Jul 11, 2017
@oising
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
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