Skip to content
This repository has been archived by the owner on Oct 25, 2022. It is now read-only.

Make directory separators platform agnostic #237

Merged
merged 1 commit into from
Jan 30, 2021

Conversation

TheCakeIsNaOH
Copy link
Contributor

This does not get everything, specifically, I did not touch the tests, or the automatic checksumming section.

For the automatic checksumming, I am kind of in favor of putting a check for non-windows, telling people to use Get-RemoteChecksum or embed, then throwing.

Also, $ENV:Temp does not exist on Linux, so I switched that to [System.IO.Path]::GetTempPath().

Part of #234

@majkinetor
Copy link
Owner

Isn't slash type irrelevant - '/' works on Windows and \ should work in pwsh on linux ? I just tested it and works fine.

I did not touch the tests

Tests should work too cross-platform if we go with this; those that are Windwos specific, if any, could be tagged.

For the automatic checksumming, I am kind of in favor of putting a check for non-windows, telling people to use Get-RemoteChecksum or embed, then throwing.

I was thinking in deprecating automatic checksum but I guess people use that a lot. So yeah, that, is a must. Also I am not sure about choco dependency in general (but lets talk about that on #234.

@TheCakeIsNaOH
Copy link
Contributor Author

Isn't slash type irrelevant - '/' works on Windows and \ should work in pwsh on linux ? I just tested it and works fine.

I had issues with NuspecPath and get-remotefiles.

Tests should work too cross-platform if we go with this; those that are Windows specific, if any, could be tagged.

I haven't been able to get them to work (yet), but that is good to know.

@@ -14,7 +14,7 @@ function Push-Package() {
[switch] $All
)
$api_key = if (Test-Path api_key) { Get-Content api_key }
elseif (Test-Path ..\api_key) { Get-Content ..\api_key }
elseif (Test-Path (Join-Path '..' 'api_key')) { Get-Content (Join-Path '..' 'api_key') }
Copy link
Owner

@majkinetor majkinetor Jan 30, 2021

Choose a reason for hiding this comment

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

Mhm.. this quickly became hard to look at... that is one of the reasons I didn't initially go with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it really does.

That could be resolved by making it into a variable.
$apikeypath = (Join-Path '..' 'api_key')

Copy link
Owner

Choose a reason for hiding this comment

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

Just ranting...

@majkinetor majkinetor merged commit 18621b1 into majkinetor:master Jan 30, 2021
@majkinetor majkinetor mentioned this pull request Jan 30, 2021
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants