-
-
Notifications
You must be signed in to change notification settings - Fork 36
docs: various tiny doc fixes #450
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR contains various documentation improvements across the Cot web framework codebase. The changes focus on improving clarity, consistency, and grammatical correctness of documentation comments, error messages, and CLI descriptions throughout the project.
Changes:
- Improved documentation phrasing for better readability and consistency (e.g., "Sends a GET request" instead of "Send a GET request")
- Corrected grammatical issues and standardized terminology (e.g., "test configuration" instead of "test config", "specifications" instead of "specs")
- Enhanced error message descriptions to be more descriptive and clear
- Updated CLI tool description to be more professional and concise
- Fixed minor typos and spelling errors in code comments and examples
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cot/src/test.rs | Improved documentation clarity for test client and methods |
| cot/src/static_files.rs | Enhanced module and macro documentation |
| cot/src/session.rs | Refined module documentation phrasing |
| cot/src/router.rs | Improved documentation for router methods and parameter descriptions |
| cot/src/response.rs | Fixed grammar and improved trait documentation |
| cot/src/request.rs | Enhanced method documentation clarity |
| cot/src/project.rs | Improved trait and method documentation, fixed comment formatting |
| cot/src/openapi.rs | Enhanced OpenAPI-related documentation |
| cot/src/middleware.rs | Fixed grammar and spelling in middleware documentation |
| cot/src/json.rs | Corrected terminology ("RESTful" instead of "REST-ful") |
| cot/src/html.rs | Improved safety documentation phrasing |
| cot/src/handler.rs | Simplified error documentation |
| cot/src/form.rs | Enhanced error message and method documentation |
| cot/src/db/migrations.rs | Improved migration engine documentation |
| cot/src/db.rs | Enhanced error documentation and method descriptions |
| cot/src/config.rs | Fixed grammar and improved configuration documentation |
| cot/src/common_types.rs | Corrected article usage and improved method documentation |
| cot/src/cli.rs | Enhanced CLI registration documentation |
| cot/src/cache/store/redis.rs | Improved Redis store documentation |
| cot/src/cache/store/memory.rs | Enhanced in-memory store documentation |
| cot/src/cache/store.rs | Fixed grammar in trait documentation |
| cot/src/cache.rs | Removed redundant documentation and fixed comment formatting |
| cot/src/body.rs | Consolidated error documentation |
| cot/src/auth/db.rs | Improved database authentication documentation |
| cot/src/auth.rs | Fixed typos and enhanced authentication trait documentation |
| cot/src/admin.rs | Updated admin trait documentation and fixed import examples |
| cot-macros/src/lib.rs | Improved macro documentation clarity |
| cot-codegen/src/symbol_resolver.rs | Fixed extra space in documentation |
| cot-cli/src/migration_generator.rs | Enhanced migration generation error messages and documentation |
| cot-cli/src/args.rs | Standardized CLI argument documentation format |
| cot-cli/Cargo.toml | Updated package description to be more professional |
| Snapshot test files | Updated to reflect new CLI description |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Returns an error if the object could not be saved, for instance | ||
| /// due to a database error. | ||
| /// Returns an error if the object could not be saved, for example, | ||
| /// a database error. |
Copilot
AI
Jan 19, 2026
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.
The error message description "for example, a database error" is grammatically incomplete. It should either be "due to a database error" (like the original) or "for example, due to a database error" to be a complete phrase.
| /// a database error. | |
| /// due to a database error. |
| /// Returns an error if the object could not be removed, for example, | ||
| /// a database error. |
Copilot
AI
Jan 19, 2026
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.
The error message description "for example, a database error" is grammatically incomplete. It should either be "due to a database error" (like the original) or "for example, due to a database error" to be a complete phrase.
| /// Returns an error if the object could not be removed, for instance | ||
| /// due to a database error. | ||
| /// Returns an error if the object could not be removed, for example, | ||
| /// a database error. |
Copilot
AI
Jan 19, 2026
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.
The error message description "for example, a database error" is grammatically incomplete. It should either be "due to a database error" (like the original) or "for example, due to a database error" to be a complete phrase.
| /// a database error. | |
| /// due to a database error. |
|
| Branch | doc-fixes |
| Testbed | github-ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result microseconds (µs) (Result Δ%) | Upper Boundary microseconds (µs) (Limit %) |
|---|---|---|---|
| empty_router/empty_router | 📈 view plot 🚷 view threshold | 5,555.00 µs(-7.96%)Baseline: 6,035.71 µs | 6,868.70 µs (80.87%) |
| json_api/json_api | 📈 view plot 🚷 view threshold | 998.92 µs(-2.20%)Baseline: 1,021.43 µs | 1,140.15 µs (87.61%) |
| nested_routers/nested_routers | 📈 view plot 🚷 view threshold | 926.69 µs(-1.82%)Baseline: 943.88 µs | 1,048.90 µs (88.35%) |
| single_root_route/single_root_route | 📈 view plot 🚷 view threshold | 863.64 µs(-4.35%)Baseline: 902.91 µs | 1,003.59 µs (86.06%) |
| single_root_route_burst/single_root_route_burst | 📈 view plot 🚷 view threshold | 16,974.00 µs(-3.19%)Baseline: 17,534.11 µs | 20,807.86 µs (81.57%) |
| bail!( | ||
| "Generating migrations for workspaces is not supported yet. \ | ||
| Please generate migrations for each package separately." | ||
| Please run the command from within a specific package directory." |
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.
I think we should retain the note that one's need to run it for each package separately.
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.
Looking at the snapshots, () are still used for defaults, so I think we should keep using them for standardization sake.
| #[non_exhaustive] | ||
| pub enum AuthError { | ||
| /// The password hash that is passed to [`PasswordHash::new`] is invalid. | ||
| /// The password hash that is provided to [`PasswordHash::new`] is invalid. |
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.
| /// The password hash that is provided to [`PasswordHash::new`] is invalid. | |
| /// The password hash that was provided to [`PasswordHash::new`] is invalid. |
| /// because the model might have changed since the migration was generated. | ||
| /// You can, however, use the migration model, which will always represent | ||
| /// the state of the model at the time the migration runs. | ||
| /// engine knows what the state of model was at the time the last migration |
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.
I feel like the whole Paragraph could be worded like so:
/// Migration models have two major uses. First, they ensure that the migration engine
/// knows what state the model was in at the time the last migration was generated.
/// This allows the engine to automatically detect the changes and generate the necessary migration code.
/// Second, they allow custom code in migrations: you might want the migration to fill in some data, for example.
/// You cannot use the actual model for this because the
/// model might have changed since the migration was generated.
/// You can, however, use the migration model, which will always represent the state of the model at the time the migration runs.
| /// custom code in the migrations: you might want the migration to fill in | ||
| /// some data, for example. You can't use the actual model for this because | ||
| /// the model might have changed since the migration was generated. You can, | ||
| /// however, use the migration model, which will always represent the state of | ||
| /// the model at the time the migration runs. |
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.
This part is not so clear. Perhaps an actual code example to illustrate what you can't do with the actual model, but can with the migration model, should help clear things up?
| /// app. | ||
| /// | ||
| /// This is pretty much an equivalent to `#[tokio::test]` provided so that you | ||
| /// This is equivalent to `#[tokio::test]`, but provided so that you |
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.
| /// This is equivalent to `#[tokio::test]`, but provided so that you | |
| /// This is equivalent to `#[tokio::test]`, but is provided so that you |
| /// the hash is obsolete. The new hash calculated with the currently | ||
| /// preferred algorithm is provided, and it should be saved to the |
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.
| /// the hash is obsolete. The new hash calculated with the currently | |
| /// preferred algorithm is provided, and it should be saved to the | |
| /// the hash is obsolete. The new hash, calculated with the currently | |
| /// preferred algorithm, is provided and should be saved to the |
| /// mounted on the project's router, its own set of middleware, database | ||
| /// migrations (which can depend on other apps), etc. | ||
| /// Each app can have its own set of URLs that can be mounted on the project's | ||
| /// router, its own set of middleware, database migrations (which can depend on |
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.
| /// router, its own set of middleware, database migrations (which can depend on | |
| /// router, its own set of middlewares, database migrations (which can depend on |
|
|
||
| /// Get the app name the current route belongs to, or [`None`] if the | ||
| /// request is not routed. | ||
| /// Returns the name of the app the current route belongs to, or [`None`] if |
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.
| /// Returns the name of the app the current route belongs to, or [`None`] if | |
| /// Returns the name of the app that the current route belongs to, or [`None`] if |
| /// Get the route name, or [`None`] if the request is not routed or doesn't | ||
| /// have a route name. | ||
| /// Returns the name of the current route, or [`None`] if the request is not | ||
| /// routed or doesn't have a route name. |
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.
| /// routed or doesn't have a route name. | |
| /// routed or does not have a route name. |
"Does not" sounds a lot formal imo.
Alternatively(and debatable), perhaps the sentence could use bullet points for clarity:
/// Returns the name of the current route, or [`None`] if:
///
/// - the request was not routed.
/// - the route has no name.
///
| } | ||
|
|
||
| /// Get a URL for a view by name. | ||
| /// Generates a URL for a view by its name. |
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.
| /// Generates a URL for a view by its name. | |
| /// Generates a URL for a view using its name. |
| } | ||
|
|
||
| /// Get a URL for a view by name. | ||
| /// Generates a URL for a view by its name. |
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.
| /// Generates a URL for a view by its name. | |
| /// Generates a URL for a view using its name. |
| /// registered apps. | ||
| /// | ||
| /// Returns `None` if the view name is not found. | ||
| /// It returns `None` if the view name is not found. |
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.
should the None here be a link ([None])as we've used in other places?
No description provided.