-
Notifications
You must be signed in to change notification settings - Fork 1
full package history #47
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: main
Are you sure you want to change the base?
Conversation
…y using a Move.package-history.toml file The `Move.package-history.toml` file * should be located in the same directory as the `Move.lock` file * contains all necessary data * can be maintained manually or by using a package-history CLI tool
….package-history.toml` files
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
… in package_history_cli/src/main.rs to be used by build.rs scripts of product repositories Crate package_history_cli has been removed
… can ignore envs specified by their alias
…d in build.rs implementations
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.
Mostly nitpicks, feel free to disregard anything you don't agree with
/// # Errors | ||
/// This method may return errors from the underlying `init()` or `update()` functions | ||
/// if there are issues reading or writing files. | ||
pub fn manage_history_file(&self, console_out: impl Fn(String)) -> anyhow::Result<()> { |
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.
instead of an impl Fn(String)
maybe it'd be better to use &mut impl std::fmt::Write
if self.move_lock_file_exists() { | ||
if self.history_file_exists() { | ||
// If the output file already exists, update it. | ||
console_out(format!("File `{move_history_path}` already exists, updating...")); | ||
self | ||
.update() | ||
.expect("Successfully updating `Move.history.json` file with `Move.lock` content"); | ||
console_out(format!( | ||
"Successfully updated`{move_history_path}` with content of `{move_lock_path}`" | ||
)); | ||
} else { | ||
// If the output file does not exist, create it. | ||
console_out(format!("File `{move_history_path}` does not exist, creating...")); | ||
self | ||
.init() | ||
.expect("Successfully creating a `Move.history.json` file with `Move.lock` content"); | ||
console_out(format!( | ||
"Successfully created file `{move_history_path}` with content of `{move_lock_path}` content" | ||
)); | ||
} | ||
} else { | ||
console_out(format!("File `{move_history_path}` does not exist, skipping...")); | ||
} | ||
Ok(()) |
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.
Reduce nesting with early returns
/// | ||
/// Doesn't check if any of the provided paths are invalid or if the `Move.lock` file cannot be parsed. | ||
/// Functions `init` and `update` will handle those checks. | ||
pub fn new(move_lock_path: &Path, history_file_path: &Path, aliases_to_ignore: Option<Vec<String>>) -> Self { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
console_out(format!("File `{move_history_path}` does not exist, creating...")); | ||
self | ||
.init() | ||
.expect("Successfully creating a `Move.history.json` file with `Move.lock` content"); |
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.
How can you be sure this doesn't fail? I'd probably just forward the error
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.
Right, has been a relict from a former version where this code has been placed in the build.rs
file
* Narrow down serde_json Value to type String to avoid the need for fn `remove_first_and_last_char_from_string` * Use anyhow `with_context` instead `context` where `format!` is used * Use `serde_json::to_string_pretty` to avoid the need for fn `to_prettified_string`
…default `aliases_to_watch` list
# Conflicts: # Cargo.toml # product_common/Cargo.toml
Description of change
product_common::PackageRegistry
now supports the full package version history. It uses aMove.history.json
file to import the version history and known environments (as being previously done withMove.lock
files).The
Move.history.json
fileMove.lock
filePackageRegistry
PackageRegistry
include_str!()
) - like the previously usedMove.lock
file had been integrated in the buildlocalnet
networks, as this would mass up the file. For this purpose theMoveHistoryManager
described below, offers aaliases_to_ignore
list which is set tolocalnet
per default.For example the
Move.history.json
for the recent (not latest) Identity package would look like this:Product
build.rs
scriptsThe
Move.history.json
for a product can be created manually or automatically using theMoveHistoryManager
inbuils.rs
scripts .See the docs of the MoveHistoryManager for further details.
An example for a build.rs script using the
MoveHistoryManager
to manage itsMove.history.json
file can be found in the Notarization - Full package history PR.Links to any relevant issues
Will fix: NOTARIZATION_PACKAGE_REGISTRY: Support full package address version history #71