Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,40 @@ jobs:
override: true

- name: Publish spawned-macros
run: cargo publish --package spawned-macros --token ${{ secrets.CRATES_IO_TOKEN }}
run: |
OUTPUT=$(cargo publish --package spawned-macros --token ${{ secrets.CRATES_IO_TOKEN }} 2>&1) || {
if echo "$OUTPUT" | grep -q "already exists"; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Grep pattern may not match actual crates.io error message

The error string "already exists" likely does not match what cargo publish actually outputs when a crate version is already published. The crates.io API returns:

crate version `spawned-macros@0.5.0` is already uploaded to crates.io

This contains "already uploaded", not "already exists". If the grep pattern never matches, the else branch will always execute on re-runs — printing the output and calling exit 1 — which defeats the entire purpose of this PR.

The same pattern is repeated for spawned-rt (line 36) and spawned-concurrency (line 50). Please verify the exact error message against the real failure and consider broadening the match to cover both possible phrasings:

Suggested change
if echo "$OUTPUT" | grep -q "already exists"; then
if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yaml
Line: 22

Comment:
**Grep pattern may not match actual crates.io error message**

The error string `"already exists"` likely does not match what `cargo publish` actually outputs when a crate version is already published. The crates.io API returns:

```
crate version `spawned-macros@0.5.0` is already uploaded to crates.io
```

This contains **"already uploaded"**, not **"already exists"**. If the grep pattern never matches, the `else` branch will always execute on re-runs — printing the output and calling `exit 1` — which defeats the entire purpose of this PR.

The same pattern is repeated for `spawned-rt` (line 36) and `spawned-concurrency` (line 50). Please verify the exact error message against the real failure and consider broadening the match to cover both possible phrasings:

```suggestion
            if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
```

How can I resolve this? If you propose a fix, please make it concise.

echo "spawned-macros already published, continuing"
else
echo "$OUTPUT"
exit 1
fi
}
Comment on lines +21 to +28
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Successful publish output is silently discarded

When cargo publish exits 0, the content captured in OUTPUT is never printed, so workflow logs give no visibility into what was published. Consider echoing the output on success for observability:

Suggested change
OUTPUT=$(cargo publish --package spawned-macros --token ${{ secrets.CRATES_IO_TOKEN }} 2>&1) || {
if echo "$OUTPUT" | grep -q "already exists"; then
echo "spawned-macros already published, continuing"
else
echo "$OUTPUT"
exit 1
fi
}
OUTPUT=$(cargo publish --package spawned-macros --token ${{ secrets.CRATES_IO_TOKEN }} 2>&1) && echo "$OUTPUT" || {
if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
echo "spawned-macros already published, continuing"
else
echo "$OUTPUT"
exit 1
fi
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yaml
Line: 21-28

Comment:
**Successful publish output is silently discarded**

When `cargo publish` exits 0, the content captured in `OUTPUT` is never printed, so workflow logs give no visibility into what was published. Consider echoing the output on success for observability:

```suggestion
          OUTPUT=$(cargo publish --package spawned-macros --token ${{ secrets.CRATES_IO_TOKEN }} 2>&1) && echo "$OUTPUT" || {
            if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
              echo "spawned-macros already published, continuing"
            else
              echo "$OUTPUT"
              exit 1
            fi
          }
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


- name: Wait for crates.io indexing
run: sleep 30

- name: Publish spawned-rt
run: cargo publish --package spawned-rt --token ${{ secrets.CRATES_IO_TOKEN }}
run: |
OUTPUT=$(cargo publish --package spawned-rt --token ${{ secrets.CRATES_IO_TOKEN }} 2>&1) || {
if echo "$OUTPUT" | grep -q "already exists"; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same grep pattern concern as spawned-macros step

The "already exists" pattern here has the same potential mismatch issue as described at line 22. If the crates.io error says "already uploaded", this check will silently fail and the step will always exit with error 1 on re-runs.

Suggested change
if echo "$OUTPUT" | grep -q "already exists"; then
if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yaml
Line: 36

Comment:
**Same grep pattern concern as `spawned-macros` step**

The `"already exists"` pattern here has the same potential mismatch issue as described at line 22. If the crates.io error says "already uploaded", this check will silently fail and the step will always exit with error 1 on re-runs.

```suggestion
            if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
```

How can I resolve this? If you propose a fix, please make it concise.

echo "spawned-rt already published, continuing"
else
echo "$OUTPUT"
exit 1
fi
}

- name: Wait for crates.io indexing
run: sleep 30

- name: Publish spawned-concurrency
run: cargo publish --package spawned-concurrency --token ${{ secrets.CRATES_IO_TOKEN }}
run: |
OUTPUT=$(cargo publish --package spawned-concurrency --token ${{ secrets.CRATES_IO_TOKEN }} 2>&1) || {
if echo "$OUTPUT" | grep -q "already exists"; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same grep pattern concern as spawned-macros step

Same potential mismatch: "already exists" vs "already uploaded". If this doesn't match, spawned-concurrency re-runs will always fail.

Suggested change
if echo "$OUTPUT" | grep -q "already exists"; then
if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yaml
Line: 50

Comment:
**Same grep pattern concern as `spawned-macros` step**

Same potential mismatch: `"already exists"` vs `"already uploaded"`. If this doesn't match, `spawned-concurrency` re-runs will always fail.

```suggestion
            if echo "$OUTPUT" | grep -qE "already uploaded|already exists"; then
```

How can I resolve this? If you propose a fix, please make it concise.

echo "spawned-concurrency already published, continuing"
else
echo "$OUTPUT"
exit 1
fi
}
Loading