-
-
Notifications
You must be signed in to change notification settings - Fork 316
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
CI(OSGeo4W): Use setup-OSGeo4W action to parametrize install options #4290
Conversation
I accidentally created the PR directly in grass, so I edited the PR description instead of leaving it empty |
9e73eae
to
253284d
Compare
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.
I like this!
I would merge this as is. We can consider what to do with echoix/setup-OSGeo4W. Perhaps it should live under OSGeo, but we do that later.
However, perhaps one of the people who worked on the workflow or Windows side in general should comment here. @ninsbl @HuidaeCho @landam @hellik
For the last three weekends I've been using it (or what it was before being finished), and it was pretty convenient to be able to change the parameters and explore it. Counting from the beginning, it approaches the 250 runs I've made with this solution and works well without hiccups |
I'm waiting for comments for a last day before merging |
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.
I can't review deeply as very limited internet access these days. The approach looks good.
Lately I’ve been working to get pytest running on Windows too. I end up needing some more changes to make it work, however using directly the same script that OSGeo4W uses brings me closer. The changes I would need would be reduced by having some common install options (including the OSGeo4W root) refactored out. I will also need #4121 in a near future too. And I find installation a bit long too.
So, I ended up creating an action similar to other setup-* actions, that easily allow to instal and configure a software or language. I longly thought out the pros and cons of having the action inside this repo or as a separate repo. I wrote to @wenzeslaus too during that.
I finally settled on creating a clean version of the action in a separate repo, for a couple reasons:
Improvements on using that action instead of jamming everything in the same step inline: