Skip to content

Add top-level exmaples to all crates with public API#2487

Merged
Sebastian Thiel (Byron) merged 2 commits intomainfrom
top-level-examples
Mar 25, 2026
Merged

Add top-level exmaples to all crates with public API#2487
Sebastian Thiel (Byron) merged 2 commits intomainfrom
top-level-examples

Conversation

@Byron
Copy link
Copy Markdown
Member

@Byron Sebastian Thiel (Byron) commented Mar 25, 2026

Tasks

  • review crate by crate
  • update crate-status.md

@Byron Sebastian Thiel (Byron) force-pushed the top-level-examples branch 3 times, most recently from 5fa5a9a to c5cceee Compare March 25, 2026 11:57
@Byron Sebastian Thiel (Byron) marked this pull request as ready for review March 25, 2026 11:57
Copy link
Copy Markdown
Contributor

@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.

Reviewed commit: c5cceeec77

ℹ️ 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 (@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 (@codex) address that feedback".

Codex (codex) and others added 2 commits March 25, 2026 20:28
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
@Byron Sebastian Thiel (Byron) merged commit 29c275e into main Mar 25, 2026
32 checks passed
Copy link
Copy Markdown
Member

@EliahKagan Eliah Kagan (EliahKagan) left a comment

Choose a reason for hiding this comment

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

The YAML coding style changes here are not applied consistently across workflow files. Those that are intentional should be applied consistently, which I'd be pleased to do. But I also wonder if some are not intentional. I've commented below with details, including some information about tradeoffs.

Comment on lines -7 to +8
- 'run-ci/**'
- '**/run-ci/**'
- "run-ci/**"
- "**/run-ci/**"
Copy link
Copy Markdown
Member

@EliahKagan Eliah Kagan (EliahKagan) Mar 25, 2026

Choose a reason for hiding this comment

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

If we're switching from single quotes to double quotes as the default quoting style in GitHub Actions workflow files, then I think we should do so consistently, rather than changing the style in just this one workflow file while leaving it as it was in the others. (I also wonder if this change may have been unintentional, which could be the case if it's auto-applied on save in an editor, or if Codex confused YAML and Rust string syntax, etc.)

I'd be happy to make the change to double quotes in our other YAML files, but I wanted to check first. Personally, I do not prefer double quotes in GitHub Actions workflows (that is, I prefer single quotes), and the main reason is that if one makes a mistake and gets confused between what is YAML and what is internally quoted and made part of a shell command, things tend not to go as wrong with single quotes. There are some other reasons I prefer single quotes in GHA workflows, one of which is noted in review comments below. This preference is nonetheless weak, and I have no real objection to the change it's intended and if we're willing to apply it to all workflow files in the repo.

defaults:
run:
shell: bash # Use `bash` even in the Windows job.
shell: bash # Use `bash` even in the Windows job.
Copy link
Copy Markdown
Member

@EliahKagan Eliah Kagan (EliahKagan) Mar 25, 2026

Choose a reason for hiding this comment

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

The two spaces before the comment has been consistently used for all comments in GitHub Actions workflows in this project other than those that provide information that is directly used by Dependabot, and in my opinion it makes it easier to tell what's a comment and what's the code. (This is inspired by the PEP-8 guideline for Python code to place two spaces before a comment that appears after code on the same line. To be clear, PEP-8 is not a style guide for YAML code; the only question is whether we judge that the reasoning also applies here.)

Regardinng the Dependabot exception, version comments associated with pinned OIDs have not had them, since the expected format for those is exactly one space. This was its own kind of unforced inconsistency. So there's an argument that we should use one space for all comments, and I don't object to this change, but I do think that, like with the quoting style change, it should be done consistently across workflow files if it is to be kept.

rustup default stable
- uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2
with:
save-if: ${{ github.ref == 'refs/heads/main' }}
Copy link
Copy Markdown
Member

@EliahKagan Eliah Kagan (EliahKagan) Mar 25, 2026

Choose a reason for hiding this comment

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

This line looks okay as it stands, regardless of the primary quoting style, but it illustrates one of the benefits of preferring single quotes.

One of the reasons I prefer single quotes over double quotes is that they seem to have slightly fewer pitfalls in GitHub Actions workflows. For example, even as single quotes were changed to double quotes elsewhere, this new code has single quotes, as if I recall correctly, it must--this should not be changed to double quotes even if others are. The reason is that, inside ${{ }}, one cannot use double quotes, if I recall correctly. I have not had to think about this subtlety very deeply, since preferring single quotes just does the right thing across more cases.

@Byron
Copy link
Copy Markdown
Member Author

The YAML coding style changes here are not applied consistently across workflow files. Those that are intentional should be applied consistently, which I'd be pleased to do. But I also wonder if some are not intentional. I've commented below with details, including some information about tradeoffs.

Eliah Kagan (@EliahKagan) Apologies for that! My editor (Zed) did it automatically and I suppose it's a good idea to style consistently. I'd love it if you could set it up. By now I have turned off auto-formatting for Markdown at least, but in general I'd love to move to a 'format automatically' rule if it makes sense for YAML as well.
Also, please feel free to undo my formatting change if it's not a good time, we can see it more as a bug.

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.

3 participants