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.

Sign up for GitHub

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

Jump to bottom

ci: Fix cache version on Windows #3119

Merged
merged 2 commits into from
Jul 27, 2023
Merged

ci: Fix cache version on Windows #3119

merged 2 commits into from
Jul 27, 2023

Conversation

max-sixty
Copy link
Member

The existing code doesn't seem to be working correctly -- maybe windows isn't running the script correctly? https://github.com/PRQL/prql/actions/runs/5673273615/job/15374368488#step:8:60

Let's try this.

@max-sixty
ci: Fix cache version on Windows
4a803d3 
The existing code doesn't seem to be working correctly -- maybe windows isn't running the script correctly? https://github.com/PRQL/prql/actions/runs/5673273615/job/15374368488#step:8:60

Let's try this.
@max-sixty
Copy link
Member Author

No luck unfortunately — https://github.com/PRQL/prql/actions/runs/5673550570/job/15375269508?pr=3119

Happy if we can resolve a different way @not-my-profile . The old approach was worse but worked, so if you prefer to revert, that's fine too.

@eitsupi
Copy link
Member

eitsupi commented Jul 26, 2023

Perhaps it has something to do with the lack of shebang?

@not-my-profile not-my-profile mentioned this pull request Jul 27, 2023
Merged
@not-my-profile
Copy link
Collaborator

Perhaps it has something to do with the lack of shebang?

I don't think Windows supports shebangs ... and the GitHub workflow doc also doesn't mention them.

@eitsupi
Copy link
Member

eitsupi commented Jul 27, 2023

I don't think Windows supports shebangs ... and the GitHub workflow doc also doesn't mention them.

If we specify shell as bash on Windows on GHA, it is bash on MinGW or something and shebangs will be used, I believe.

@not-my-profile
Copy link
Collaborator

Yes but at that point it's already running in bash so we don't need a shebang anymore.

@richb-hanover richb-hanover mentioned this pull request Jul 27, 2023
Open
not-my-profile
not-my-profile approved these changes Jul 27, 2023
View reviewed changes
Copy link
Collaborator

@not-my-profile not-my-profile left a comment

Choose a reason for hiding this comment

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

LGTM

@max-sixty max-sixty merged commit a9db1e2 into PRQL:main Jul 27, 2023
43 checks passed
@max-sixty max-sixty deleted the cache-version branch July 27, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@not-my-profile not-my-profile not-my-profile approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@max-sixty @eitsupi @not-my-profile