-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Package only from root plus powershell #4569
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
Package only from root plus powershell #4569
Conversation
tools/packaging/packaging.psm1
Outdated
Write-Warning "Skipping release checks." | ||
} | ||
|
||
if(!$Script:Options.RootInfo.IsValid -and -not $SkipReleaseChecks.IsPresent){ |
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.
an elseif
is better here.
elseif (!$Script:Options.RootInfo.IsValid) {
throw $Script:Options.RootInfo.Warning
}
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.
Resolved
build.psm1
Outdated
{ | ||
$RootInfo += @{Warning = "Please ensure you repo is at the root of the file system and named 'PowerShell' (example: '$($RootInfo.ValidPath)'), when building and packaging for release!" } | ||
$RootInfo += @{IsValid = $false} | ||
Write-Warning -Message $RootInfo.Warning |
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.
Don't write the warning in New-PSOptions
. We only care about the root path when it comes to a release build.
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.
Resolved
tools/travis.ps1
Outdated
$packages += Start-PSPackage @packageParams -Type AppImage | ||
$packages = @(Start-PSPackage @packageParams -SkipReleaseChecks) | ||
# Packaging AppImage depends on the deb package | ||
$packages += Start-PSPackage @packageParams -Type AppImage -SkipReleaseChecks |
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.
There is an extra space before -SkipReleaseChecks
, in both line 182 and 184
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.
Resolved
build.psm1
Outdated
$RootInfo = @{RepoPath = $PSScriptRoot} | ||
|
||
# the valid root is the root of the filesystem and the folder PowerShell | ||
$RootInfo += @{ValidPath = Join-Path -Path ([system.io.path]::GetPathRoot($RootInfo.RepoPath)) -ChildPath 'PowerShell' } |
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.
A minor comment about the way you use Hashtable: in this scenario it's better to write script like
$RootInfo['ValidPath'] = Join-Path -Path ([system.io.path]::GetPathRoot($RootInfo.RepoPath)) -ChildPath 'PowerShell'
$RootInfo['IsValid'] = $true
$RootInfo['Warning'] = "Please ensure you repo is at the root of the file system and named 'PowerShell' (example: '$($RootInfo.ValidPath)'), when building and packaging for release!"
It's much less expensive than adding two hashtables.
fixes #4492