search

ericsink / SQLitePCL.raw

A Portable Class Library (PCL) for low-level (raw) access to SQLite
Apache License 2.0
512 stars 106 forks source link

Make Nuspec path platform independent #487

Closed kant2002 closed 2 years ago

kant2002 commented 2 years ago

When I run build on Linux, paths was changed. In Nuspec by convention prefered to always use \. Without that change I have noise after running build.

ericsink commented 2 years ago

In intentionally use Path.Combine() because it seems less prone to introducing bugs than having string manipulation at every call site.

But I do see your point about the convention of using backslashes in a nuspec. I'm using System.IO.Path.Combine(), but this is not a system path.

I'm not sure what to do with this yet. For now, thanks for the PR.

kant2002 commented 2 years ago

I cannot find exact statement in the Nuget docs, but same as MSBuild they allow use Windows paths and translate them to OS-specific paths behind the scene. I would call these paths, not real paths which OS consume, but some sort of virtual paths which is used by Nuget to find files.

It's not super important for me, since it's only 2 files which I can manually ignore when commit. Just think that this make things nicer when working from Linux (which I occasionally do)

ericsink commented 2 years ago

I do agree in principle. I just wish the string manipulation were wrapped in a function so it only happened in one place.

Like maybe a NuGetPath.Combine(), which always uses backslash instead of the OS-specific path separator.

kant2002 commented 2 years ago

@ericsink can you take a look?

ericsink commented 2 years ago

Seems to look okay. CI is running build/tests.

kant2002 commented 2 years ago

Build seems to be okay. Anything else left from my side, so this can be merged?

ericsink commented 2 years ago

It's probably ready. I'll merge it pretty soon.