Skip to content
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

feat: add upgrading info outputs to internal-prepare #374

Conversation

ncatelli
Copy link
Contributor

@ncatelli ncatelli commented Apr 9, 2024

General

This PR starts the work of improving the logging discussed on #372 by adding explicit logging of packages and dependencies that have changed.

Below I will include examples as they are implemented for open discussion

TODO

  • info level logging of changes to packages in Prepare stage
  • info level logging of published packages in Publish stage
  • debug trace level logging of cargo commands executed in Publish stage
    • This was already implemented
  • Increase verify-conditions info log level to debug
  • Add a log-level flag that's mutually-exclusive, and overriding of, the verbosity flag

Examples

Verify

Info Level

$ ../../target/release/semantic-release-cargo -v prepare 2.0.0
semantic_release_cargo: Building package graph
semantic_release_cargo: Setting version information for packages in the workspace.
semantic_release_cargo: Setting the version of dependencies to 2.0.0
semantic_release_cargo: Upgrading dependency of dep1 to dependencies@2.0.0
semantic_release_cargo: Upgrading build-dependency of build1 to dependencies@2.0.0
semantic_release_cargo: Setting the version of build1 to 2.0.0
semantic_release_cargo: Setting the version of dep1 to 2.0.0

Log Level Flag

$ ../../target/release/semantic-release-cargo --log-level=info prepare 2.0.0
semantic_release_cargo: Setting the version of dependencies to 2.0.0
semantic_release_cargo: Upgrading dependency of dep1 to dependencies@2.0.0
semantic_release_cargo: Upgrading build-dependency of build1 to dependencies@2.0.0
semantic_release_cargo: Setting the version of build1 to 2.0.0
semantic_release_cargo: Setting the version of dep1 to 2.0.0
$ ../../target/release/semantic-release-cargo --log-level=debug prepare 2.0.0
semantic_release_cargo: Building package graph
semantic_release_cargo: manifest_path: None
semantic_release_cargo: Setting version information for packages in the workspace.
semantic_release_cargo: reading /workspaces/semantic-release-cargo/test_data/dependencies/Cargo.toml
semantic_release_cargo: Setting the version of dependencies to 2.0.0
semantic_release_cargo: Upgrading dependency of dep1 to dependencies@2.0.0
semantic_release_cargo: Upgrading build-dependency of build1 to dependencies@2.0.0
semantic_release_cargo: writing /workspaces/semantic-release-cargo/test_data/dependencies/Cargo.toml
semantic_release_cargo: reading /workspaces/semantic-release-cargo/test_data/dependencies/Cargo.lock
semantic_release_cargo: writing /workspaces/semantic-release-cargo/test_data/dependencies/Cargo.lock
semantic_release_cargo: reading /workspaces/semantic-release-cargo/test_data/dependencies/build1/Cargo.toml
semantic_release_cargo: Setting the version of build1 to 2.0.0
semantic_release_cargo: writing /workspaces/semantic-release-cargo/test_data/dependencies/build1/Cargo.toml
semantic_release_cargo: reading /workspaces/semantic-release-cargo/test_data/dependencies/dep1/Cargo.toml
semantic_release_cargo: Setting the version of dep1 to 2.0.0
semantic_release_cargo: writing /workspaces/semantic-release-cargo/test_data/dependencies/dep1/Cargo.toml

Other Changes

  • Updates info log-level to output to stdout

@ncatelli
Copy link
Contributor Author

ncatelli commented Apr 9, 2024

Is there a reason we double-output errors in the verify-condition stage?

return writeln!(
output,
"Unable to build workspace dependencies graph: {}",
err
)
.map_err(|io_error| -> anyhow::Error { Error::output_error(io_error).into() })
.and(Err(err));
}

It ends up double outputting

$ ../../target/release/semantic-release-cargo verify-conditions
registry token for crates-io empty or not set.
Error: Conditions for a release are not satisfied: registry token for crates-io empty or not set.

I'm wondering if it would be better to remove the writeln calls in favor of the single error point.

@ncatelli ncatelli marked this pull request as ready for review April 9, 2024 20:06
@ncatelli
Copy link
Contributor Author

ncatelli commented Apr 9, 2024

Open questions

  • There is some inconsistency between capitalized logs. I'd assume we'd want to standardize on one or the other
  • I changed the log level for Info to output to stdout out, should we also remove the exception for the other log levels?

@EricCrosson
Copy link
Contributor

Is there a reason we double-output errors in the verify-condition stage?

There is no good reason, please feel free to simplify!

@EricCrosson
Copy link
Contributor

I changed the log level for Info to output to stdout out, should we also remove the exception for the other log levels?

I'm not entirely sure what consequence this has, but I assume if it's safe to do for info, it's safe to do for debug and trace.

@EricCrosson
Copy link
Contributor

There is some inconsistency between capitalized logs. I'd assume we'd want to standardize on one or the other

Looks like you standardized to sentence-case logs, looks good to me 👍

Copy link
Contributor

@EricCrosson EricCrosson left a comment

Choose a reason for hiding this comment

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

Prints log lines more verbosely and consistently ✨

Thank you @ncatelli!

@EricCrosson EricCrosson merged commit 09132d3 into semantic-release-cargo:master Apr 9, 2024
11 checks passed
Copy link

github-actions bot commented Apr 9, 2024

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ncatelli ncatelli deleted the feat/372/improve-logging-on-subcommands branch April 10, 2024 02:00
@ncatelli
Copy link
Contributor Author

Is there a reason we double-output errors in the verify-condition stage?

There is no good reason, please feel free to simplify!

Perfect, I may come through with a follow-up PR for that and the log targets in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants