Skip to content
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

Clean up some error messages #2032

Merged
merged 1 commit into from
Mar 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions bin/auto-pr.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,15 @@ if ((!$push -and !$request) -or $help) {
}

if(!($upstream -match "^(.*)\/(.*):(.*)$")) {
Write-Host -f DarkRed "Upstream must have this format: <user>/<repo>:<branch>"
exit 1
abort "Upstream must have this format: <user>/<repo>:<branch>"
}

function execute($cmd) {
Write-Host -f Green $cmd
$output = iex $cmd

if($LASTEXITCODE -gt 0) {
Write-Host -f Red "^^^ Error! See above ^^^ (last command: $cmd)"
exit 1
abort "^^^ Error! See above ^^^ (last command: $cmd)"
}
return $output
}
Expand Down Expand Up @@ -91,7 +89,7 @@ function pull_requests($json, [String]$app, [String]$upstream, [String]$manifest
execute "hub push origin $branch"

if($LASTEXITCODE -gt 0) {
Write-Host -f DarkRed "Push failed! (hub push origin $branch)"
error "Push failed! (hub push origin $branch)"
execute "hub reset"
return
}
Expand All @@ -107,9 +105,8 @@ function pull_requests($json, [String]$app, [String]$upstream, [String]$manifest
$msg += "</table>"
hub pull-request -m "$msg" -b '$upstream' -h '$branch'
if($LASTEXITCODE -gt 0) {
Write-Host -f DarkRed "Pull Request failed! (hub pull-request -m 'Update $app to version $version' -b '$upstream' -h '$branch')"
execute "hub reset"
exit 1
abort "Pull Request failed! (hub pull-request -m 'Update $app to version $version' -b '$upstream' -h '$branch')"
}
}

Expand Down Expand Up @@ -139,7 +136,7 @@ hub diff --name-only | % {
$app = ([System.IO.Path]::GetFileNameWithoutExtension($manifest))
$json = parse_json $manifest
if(!$json.version) {
Write-Host -f Red "Invalid manifest: $manifest ..."
error "Invalid manifest: $manifest ..."
return
}
$version = $json.version
Expand Down
2 changes: 1 addition & 1 deletion bin/checkver.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ $queue | % {

if ($json.checkver -eq "github") {
if (!$json.homepage.StartsWith("https://github.com/")) {
write-host "ERROR: $name checkver expects the homepage to be a github repository" -f DarkYellow
error "$name checkver expects the homepage to be a github repository"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though the original was darkyellow text, changed to error instead of warn, since it seemed like an error condition.

}
$url = $json.homepage + "/releases/latest"
$regex = $githubRegex
Expand Down
1 change: 0 additions & 1 deletion lib/psmodules.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ function install_psmodule($manifest, $dir, $global) {
$module_name = $psmodule.name
if(!$module_name) {
abort "Invalid manifest: The 'name' property is missing from 'psmodule'."
return
}

$linkfrom = "$modulesdir\$module_name"
Expand Down
4 changes: 2 additions & 2 deletions libexec/scoop-depends.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ reset_aliases
$opt, $apps, $err = getopt $args 'a:' 'arch='
$app = $apps[0]

if(!$app) { error 'ERROR: <app> missing'; my_usage; exit 1 }
if(!$app) { error '<app> missing'; my_usage; exit 1 }

$architecture = default_architecture
try {
$architecture = ensure_architecture ($opt.a + $opt.arch)
} catch {
error "ERROR: $_"; exit 1
abort "ERROR: $_"
Copy link
Member

Choose a reason for hiding this comment

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

This is fine, but kinda confusing. Could abort either not prefix 'ERROR: ', or use a different prefix, like 'FATAL: '?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could, but I wouldn't worry too much about it. As @r15ch13 said, the whole error handling needs to be changed anyway. The worst cases ("ERROR ERROR...") have been taken care of.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. thx.

}

$deps = @(deps $app $architecture)
Expand Down
6 changes: 3 additions & 3 deletions libexec/scoop-install.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ $architecture = default_architecture
try {
$architecture = ensure_architecture ($opt.a + $opt.arch)
} catch {
error "ERROR: $_"; exit 1
abort "ERROR: $_"
}

if(!$apps) { error 'ERROR: <app> missing'; my_usage; exit 1 }
if(!$apps) { error '<app> missing'; my_usage; exit 1 }

if($global -and !(is_admin)) {
error 'ERROR: you need admin rights to install global apps'; exit 1
abort 'ERROR: you need admin rights to install global apps'
}

if(is_scoop_outdated) {
Expand Down
2 changes: 1 addition & 1 deletion libexec/scoop-reset.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ reset_aliases
$opt, $apps, $err = getopt $args
if($err) { "scoop reset: $err"; exit 1 }

if(!$apps) { error 'ERROR: <app> missing'; my_usage; exit 1 }
if(!$apps) { error '<app> missing'; my_usage; exit 1 }

if($apps -eq '*') {
$local = installed_apps $false | % { ,@($_, $false) }
Expand Down
2 changes: 1 addition & 1 deletion libexec/scoop-virustotal.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ $apps | % {
}
else {
if ($_.Exception.Message -match "\(204|429\)") {
abort("$app`: VirusTotal request failed`: $($_.Exception.Message)", $exit_code)
abort "$app`: VirusTotal request failed`: $($_.Exception.Message)" $exit_code
}
warn("$app`: VirusTotal request failed`: $($_.Exception.Message)")
}
Expand Down