-
Notifications
You must be signed in to change notification settings - Fork 242
Convert tabs to spaces #1538
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
Convert tabs to spaces #1538
Conversation
Reviewer's GuideThis PR replaces tab characters with spaces for indentation within the .github/workflows/ci.yml run sections, standardizing the CI workflow formatting. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
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.
Hey @ericcurtin - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/workflows/ci.yml:106` </location>
<code_context>
run: |
- mkdir /mnt/tmp
- export TMPDIR=/mnt/tmp
+ mkdir /mnt/tmp
+ export TMPDIR=/mnt/tmp
make validate
</code_context>
<issue_to_address>
Use mkdir -p for idempotent directory creation
`mkdir -p /mnt/tmp` will prevent errors if the directory already exists, ensuring the step doesn't fail on reruns.
Suggested implementation:
```
mkdir -p /mnt/tmp
```
```
mkdir -p /mnt/tmp
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
.github/workflows/ci.yml
Outdated
@@ -103,8 +103,8 @@ jobs: | |||
|
|||
- name: run bats | |||
run: | | |||
mkdir /mnt/tmp | |||
export TMPDIR=/mnt/tmp | |||
mkdir /mnt/tmp |
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.
suggestion (bug_risk): Use mkdir -p for idempotent directory creation
mkdir -p /mnt/tmp
will prevent errors if the directory already exists, ensuring the step doesn't fail on reruns.
Suggested implementation:
mkdir -p /mnt/tmp
mkdir -p /mnt/tmp
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.
LGTM
Saw this in github ui Signed-off-by: Eric Curtin <[email protected]>
/mnt doesn't exist on macOS:
is more portable |
Merging this because the build is broken, it's more important to get the build green again |
@rhatdan just checking you didn't pick /mnt for a specific reason? Hoping it didn't have any extra disk space or something, merged this quickly as the mkdir /mnt/tmp broke our macOS build |
The issue is on Linux systems, the /mnt is a large amount of space as I understand it. If you look at the docker one, it says:
So for at lease Linux tests we should be setting TMPDIR to point at /mnt. |
@rhatdan maybe it works... We might need to if around it for macOS though, if definitely broke macOS... But there's so much instability now, it's hard to keep track |
Saw this in github ui
Summary by Sourcery
CI: