Skip to content

Comments

pin ruff commit hashes when copying docs#1636

Open
oconnor663 wants to merge 2 commits intomainfrom
ruff_commit_in_links
Open

pin ruff commit hashes when copying docs#1636
oconnor663 wants to merge 2 commits intomainfrom
ruff_commit_in_links

Conversation

@oconnor663
Copy link
Contributor

@oconnor663 oconnor663 commented Nov 25, 2025

This is a drive-by fix/idea. I noticed that we're publishing links with line numbers that tend to go stale as the upstream Rust code is edited over time (like most of the "View source" links on this page). Might be nice to snapshot these when we vendor the .md files?

@oconnor663 oconnor663 requested a review from carljm November 25, 2025 21:18
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

copy_docs() {
src="$1"
dest="$2"
[[ -d "$dest" ]] && dest="$dest/$(basename "$src")"

Choose a reason for hiding this comment

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

P1 Badge POSIX sh script now uses bash-specific conditional

The new copy_docs uses [[ -d "$dest" ]] under a /usr/bin/env sh shebang. On systems where sh is dash (Debian/Ubuntu default), [[ is unsupported and the script now aborts with a syntax error before copying any docs. Use POSIX [ -d "$dest" ] or switch the shebang to bash to keep the tool runnable.

Useful? React with 👍 / 👎.

@oconnor663
Copy link
Contributor Author

I guess if we did want to do this, we'd need to remove the CI check that specifically tests we didn't change anything...

@oconnor663
Copy link
Contributor Author

Adding @zanieb as a reviewer. Low priority, no rush, etc. I'm curious to get your take as the CEO-at-large of Rooster :)

@carljm carljm requested a review from MichaReiser November 25, 2025 23:42
}
copy_docs ./ruff/crates/ty/docs/cli.md ./docs/reference/
copy_docs ./ruff/crates/ty/docs/configuration.md ./docs/reference/
copy_docs ./ruff/crates/ty/docs/rules.md ./docs/reference/
Copy link
Member

Choose a reason for hiding this comment

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

I think we only need this for rules.md, no other markdown file contains line-references

@MichaReiser
Copy link
Member

Hmm, you now run into issues with our check-generated files unedited. That script will require the same pre-processing

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