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

Bring MacOS/Windows back into CI #486

Merged
merged 3 commits into from
May 10, 2023
Merged

Bring MacOS/Windows back into CI #486

merged 3 commits into from
May 10, 2023

Conversation

WhiffleFish
Copy link
Member

No description provided.

@WhiffleFish WhiffleFish marked this pull request as draft May 6, 2023 02:29
@zsunberg
Copy link
Member

zsunberg commented May 6, 2023

*sigh* now we have to make all of our gh actions scripts windows-proof. Yet another man-hour of humanity's time that Microsoft has wasted.

@WhiffleFish WhiffleFish marked this pull request as ready for review May 8, 2023 19:20
@zsunberg
Copy link
Member

zsunberg commented May 8, 2023

@WhiffleFish this is amazing. Can you add a comment to the yml file explaining that the conversion of symbols to strings is to maintain OS compatibility?

@lassepe
Copy link
Member

lassepe commented May 8, 2023

I understand why we need joinpath but I don't understand why we need to convert Symbols to string instead of using literal strings directly? For example, isn't string(:lib) === "lib" irrespective of the OS?

@mykelk
Copy link
Member

mykelk commented May 8, 2023

Maybe because it is in a quoted shell command?

@zsunberg
Copy link
Member

zsunberg commented May 9, 2023

I assume that it has to do with the way windows handles various quotes, so you can't use the trick where ' are used to create arguments with " in them. Thus, we have to avoid using " anywhere in the command.

@WhiffleFish is this correct? Can you add the comment requested above so that I can merge this?

@WhiffleFish
Copy link
Member Author

WhiffleFish commented May 10, 2023

Yep, that is entirely correct. I figured there'd be a way to escape the quotes (\") in a way that would work across operating systems, but haven't been able to find a way to do so.

@zsunberg
Copy link
Member

@WhiffleFish thanks! It seems like there should be a recommended way that is more straightforward, but this is great for now.

@zsunberg zsunberg merged commit cb5663a into master May 10, 2023
@zsunberg zsunberg deleted the WhiffleFish-patch-1 branch May 10, 2023 15:09
johannes-fischer pushed a commit to johannes-fischer/POMDPs.jl that referenced this pull request Jul 12, 2023
* Bring MacOS/Windows back into CI

* horrendous os-agnostic ci script

* ci string compatibility workaround comment
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.

4 participants