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

fix: check for scripts dir in project root instead of /scripts #43

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

rktjmp
Copy link
Contributor

@rktjmp rktjmp commented Aug 11, 2023

Seems this check would always fail except maybe in some CI setups.

@kdheepak
Copy link
Owner

Thanks for the fix. I seem to remember there’s a reason we did it this way. I’ll have to check the git history to confirm.

@kdheepak
Copy link
Owner

I think the issue was that for github actions to work, this is run in a Docker instance, and in Docker the scripts folder is mounted to /scripts/

COPY scripts/ /scripts/

But looking at the rest of the Dockerfile, I don't see why it wouldn't work.

I'm happy to merge and see if it works. If it doesn't I might have to revert again.

@kdheepak kdheepak merged commit e297ec6 into kdheepak:main Aug 12, 2023
1 check passed
@rktjmp
Copy link
Contributor Author

rktjmp commented Aug 12, 2023

Perhaps it could do with a comment explaining the root prefix is intentionally for when in docker/gh-actions? Or maybe check for $GITHUB_ACTION to make the intention explicit in code?

@kdheepak
Copy link
Owner

@rktjmp please check #53 as the code change from this PR has been updated!

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.

2 participants