Skip to content

Version refactor#307

Merged
DanielVoogsgerd merged 6 commits into
mainfrom
version-refactor
Jan 5, 2026
Merged

Version refactor#307
DanielVoogsgerd merged 6 commits into
mainfrom
version-refactor

Conversation

@DanielVoogsgerd

@DanielVoogsgerd DanielVoogsgerd commented Nov 10, 2025

Copy link
Copy Markdown
Collaborator

This refactor removes the homegrown Version struct and replaces it with the semver crate. Moreover, we split versions into three different versions.

BraneVersion, version numbers with respect to Brane. It is a bit unclear how we should go about the nightly and latest aliases.

AliasPackageVersion, supports semver version or the "latest" alias

ConcretePackageVersion, is a type alias for the semver crate. This could be replaced with a newtype if we so desire.

The advantage for this distinction is that an API can make clear wheter or not it accepts aliases like latest.

github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 10, 2025
View reviewed changes
Comment thread brane-ast/src/traversals/print/ast.rs
indent!(indent),
def.package,
if !def.version.is_latest() { format!("<{}>", def.version) } else { String::new() },
// FIXME: Removed latest alias

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Comment thread brane-ast/src/traversals/print/ast.rs
"{}{}::{}",
def.package,
if !def.version.is_latest() { format!("<{}>", def.version) } else { String::new() },
// FIXME:: Removed latest

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Comment thread brane-ast/src/traversals/print/ast_unresolved.rs
indent!(indent),
t.package_name,
if !t.package_version.is_latest() { format!("<{}>", t.package_version) } else { String::new() },
// FIXME: Removed latest

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Comment thread brane-ast/src/traversals/print/ast_unresolved.rs
} else {
String::new()
}
// FIXME: Unnecessary unwrap (probably)

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Comment thread brane-ast/src/traversals/print/ast_unresolved.rs
"{}{}::{}",
task.package_name,
if !task.package_version.is_latest() { format!("<{}>", task.package_version) } else { String::new() },
// FIXME: Removed latest

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Comment thread brane-cli/src/registry.rs Fixed
Comment thread brane-ctl/src/lifetime.rs Fixed
Comment thread brane-ctl/src/lifetime.rs Fixed
Comment thread brane-ctl/src/packages.rs Fixed
Comment thread specifications/src/version.rs Fixed
@codecov

codecov Bot commented Nov 10, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 7.64706% with 314 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
brane-cli/src/packages.rs 0.00% 104 Missing ⚠️
brane-ctl/src/lifetime.rs 0.00% 39 Missing ⚠️
specifications/src/version.rs 10.00% 27 Missing ⚠️
brane-ctl/src/packages.rs 0.00% 26 Missing ⚠️
brane-cli/src/utils.rs 0.00% 25 Missing ⚠️
brane-cli/src/registry.rs 0.00% 24 Missing ⚠️
specifications/src/package.rs 29.41% 11 Missing and 1 partial ⚠️
brane-api/src/schema.rs 0.00% 8 Missing ⚠️
brane-cli/src/test.rs 0.00% 7 Missing ⚠️
brane-shr/src/fs.rs 0.00% 6 Missing ⚠️
... and 11 more

📢 Thoughts on this report? Let us know!

Lut99
Lut99 requested changes Nov 12, 2025
View reviewed changes

@Lut99 Lut99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm sorry my man >.< not fun work! I've left a few comments with questions, and yeah, I'm also confused half of the time haha. Check them out, see what you think and let me know if you want to review me again (or merge when confident you've handled them all!)

Comment thread brane-api/src/schema.rs Outdated
Comment thread brane-ast/src/errors.rs Outdated
Comment thread brane-ast/src/traversals/flatten.rs
Comment thread specifications/src/wir/mod.rs
Comment thread brane-cli/src/packages.rs Outdated
Comment thread brane-ctl/src/lifetime.rs Outdated
Comment thread brane-ctl/src/lifetime.rs Outdated
Comment thread brane-ctl/src/packages.rs
Comment thread brane-ctl/src/upgrade.rs Outdated
Comment thread brane-exe/src/spec.rs Outdated
github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 17, 2025
View reviewed changes
Comment thread brane-cli/src/main.rs Fixed
Comment thread brane-cli/src/registry.rs Dismissed
Comment thread brane-ctl/src/packages.rs Dismissed
github-advanced-security[bot]
github-advanced-security AI found potential problems Dec 17, 2025
View reviewed changes
Comment thread brane-cli/src/main.rs Fixed
Comment thread brane-ctl/src/lifetime.rs Fixed
Comment thread brane-ctl/src/lifetime.rs Fixed
@DanielVoogsgerd DanielVoogsgerd force-pushed the version-refactor branch 2 times, most recently from a87baf5 to a336eac Compare December 17, 2025 15:07
Lut99
Lut99 approved these changes Dec 22, 2025
View reviewed changes

@Lut99 Lut99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Phew what a lot of work!

I left a couple of points for you to consider, but I think in general, I trust that this will work. And else we won't learn until we run it, anyway. So I'd say merge.

Comment thread Dockerfile.rls
Comment thread brane-cli/src/errors.rs Outdated
Comment thread brane-cli/src/errors.rs
Comment thread brane-cli/src/errors.rs Outdated
Comment thread brane-cli/src/packages.rs
Comment thread brane-ctl/src/upgrade.rs
Comment thread brane-shr/Cargo.toml
Comment thread docker-compose-central.yml
Comment thread docker-compose-central.yml
Comment thread specifications/src/version.rs Outdated
@DanielVoogsgerd DanielVoogsgerd merged commit 2ccd95b into main Jan 5, 2026
21 of 22 checks passed
@DanielVoogsgerd DanielVoogsgerd deleted the version-refactor branch January 5, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lut99 Lut99 Lut99 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DanielVoogsgerd @Lut99 @github-advanced-security