Skip to content

Conversation

@m4tx
Copy link
Member

@m4tx m4tx commented Jan 19, 2026

No description provided.

@github-actions github-actions bot added C-cli Crate: cot-cli (issues and Pull Requests related to Cot CLI) C-lib Crate: cot (main library crate) C-macros Crate: cot-macros C-codegen Crate: cot-codegen labels Jan 19, 2026
Copy link

Copilot AI left a 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.
Copy link

Copilot AI Jan 19, 2026

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.

Suggested change
/// a database error.
/// due to a database error.

Copilot uses AI. Check for mistakes.
Comment on lines +615 to +616
/// Returns an error if the object could not be removed, for example,
/// a database error.
Copy link

Copilot AI Jan 19, 2026

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.

Copilot uses AI. Check for mistakes.
/// 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.
Copy link

Copilot AI Jan 19, 2026

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.

Suggested change
/// a database error.
/// due to a database error.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

🐰 Bencher Report

Branchdoc-fixes
Testbedgithub-ubuntu-latest
Click to view all benchmark results
BenchmarkLatencyBenchmark 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%)
🐰 View full continuous benchmarking report in Bencher

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."
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 should retain the note that one's need to run it for each package separately.

Copy link
Member

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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// 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
Copy link
Contributor

@ElijahAhianyo ElijahAhianyo Jan 19, 2026

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.

Comment on lines +79 to +83
/// 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.
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// This is equivalent to `#[tokio::test]`, but provided so that you
/// This is equivalent to `#[tokio::test]`, but is provided so that you

Comment on lines +543 to +544
/// the hash is obsolete. The new hash calculated with the currently
/// preferred algorithm is provided, and it should be saved to the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// 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.
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-cli Crate: cot-cli (issues and Pull Requests related to Cot CLI) C-codegen Crate: cot-codegen C-lib Crate: cot (main library crate) C-macros Crate: cot-macros

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants