Skip to content

scripts: remove obsolete cargo-fmt.sh#7036

Merged
joncinque merged 2 commits intoanza-xyz:masterfrom
puhtaytow:scripts-cargo-fmt-add-sort-0
Jul 23, 2025
Merged

scripts: remove obsolete cargo-fmt.sh#7036
joncinque merged 2 commits intoanza-xyz:masterfrom
puhtaytow:scripts-cargo-fmt-add-sort-0

Conversation

@puhtaytow
Copy link
Copy Markdown

@puhtaytow puhtaytow commented Jul 18, 2025

Problem

As per #7036 (comment) this script is obsolete.

Summary of Changes

The cargo-fmt.sh is no more.

@mergify mergify Bot requested a review from a team July 18, 2025 17:01
@0xbrw 0xbrw requested a review from joncinque July 22, 2025 17:23
@0xbrw 0xbrw added the CI Pull Request is ready to enter CI label Jul 22, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Jul 22, 2025
@brooksprumo
Copy link
Copy Markdown

Personally this change falls under the "surprising" category for me. This script, based on its name, clearly is meant to communicate "I'm calling cargo-fmt for you". This PR adds a second invocation of something other than cargo-fmt, which is unexpected.

I'd be more OK with a different script calling fmt and sort, but I don't think we should call sort here.

@alexpyattaev
Copy link
Copy Markdown

Based on the discord convo here, seems like
ci/test-checks.sh is the way to go. Suggest we axe this script completely.

@puhtaytow puhtaytow changed the title scripts: add cargo sort to the cargo-fmt.sh scripts: remove obsolete cargo-fmt.sh Jul 23, 2025
@puhtaytow
Copy link
Copy Markdown
Author

Personally this change falls under the "surprising" category for me.

😅

Thank you guys for taking action on this one. I'll follow #devops channel since now on.

@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Jul 23, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Jul 23, 2025
Copy link
Copy Markdown

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Looks good to me

@joncinque joncinque merged commit b2cd2c1 into anza-xyz:master Jul 23, 2025
35 checks passed
@puhtaytow
Copy link
Copy Markdown
Author

Thank you 🙏

@puhtaytow puhtaytow deleted the scripts-cargo-fmt-add-sort-0 branch July 24, 2025 05:05
puhtaytow added a commit to puhtaytow/agave that referenced this pull request Jul 24, 2025
* add sort workspace to cargo fmt script

* remove obsolete formatting script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants