Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5 +/- ##
==========================================
- Coverage 96.88% 96.86% -0.03%
==========================================
Files 13 14 +1
Lines 1284 1403 +119
==========================================
+ Hits 1244 1359 +115
- Misses 40 44 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85ba10d0d7
ℹ️ 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".
| name = "darwen" | ||
| description = "A relational algebra library for Rust" | ||
| version = "1.1.1" | ||
| version = "1.2.0" |
There was a problem hiding this comment.
Bump major version for expanded public enum
This release adds new Predicate variants (Not, Or, Lt in src/types/predicate.rs), which is a source-breaking API change for downstream crates that exhaustively match on Predicate; publishing it as 1.2.0 can therefore break existing users on a minor upgrade. Please release this as a major version (or otherwise preserve compatibility) so dependent builds are not unexpectedly broken.
Useful? React with 👍 / 👎.
| impl From<AttributeName> for Expression { | ||
| fn from(attr: AttributeName) -> Self { | ||
| Expression::Attribute(attr) |
There was a problem hiding this comment.
Disambiguate String constants in predicate constructors
Because AttributeName is a type alias for String, this From<AttributeName> impl effectively makes every owned String become Expression::Attribute; with the new Predicate::eq/gt/lt helpers, a call like Predicate::eq(AttributeName::from("city"), city_name_string) is interpreted as attribute-vs-attribute and can return Error::InvalidAttribute instead of comparing to a string constant. This is a runtime footgun introduced by the new conversion path and will surprise callers using dynamic string values.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deadd0cb3b
ℹ️ 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".
| #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
| pub struct AttributeName(String); |
There was a problem hiding this comment.
Preserve semver compatibility for AttributeName
Replacing AttributeName from a pub type alias of String to an opaque newtype changes the public API in a source-incompatible way: downstream code that relied on String behavior (as_str, push_str, passing AttributeName to APIs expecting String, trait bounds implemented by String, etc.) will stop compiling. Because this release only bumps from 1.1.1 to 1.2.0, the change is a breaking semver regression unless you either restore string-compatible trait surface (for example via conversion/borrowing traits) or ship it as a major version bump.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deadd0cb3b
ℹ️ 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".
| /// The name of an attribute in a heading or tuple. | ||
| pub type AttributeName = String; | ||
| #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
| pub struct AttributeName(String); |
There was a problem hiding this comment.
Preserve backward compatibility for AttributeName
Replacing AttributeName from a String alias to a newtype here is a breaking public API change for existing 1.x users (for example, code that previously passed String/&String directly to Expression::Attribute, Tuple::get, or Heading::get no longer compiles), but the package version in this commit is only bumped to 1.2.0. That means downstream crates on darwen = "1" can receive compile-time breakage on a normal minor upgrade, so this needs either compatibility shims or a major-version release.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
No description provided.