-
Notifications
You must be signed in to change notification settings - Fork 265
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
chore(xtask): Upgrade xshell #4358
Conversation
Gets rid of an unexpected_cfgs warning. Signed-off-by: Kévin Commaille <[email protected]>
xtask/src/release.rs
Outdated
@@ -74,7 +74,8 @@ impl ReleaseArgs { | |||
// | |||
// More info: https://git-cliff.org/docs/usage/monorepos | |||
if self.cmd != ReleaseCommand::Changelog { | |||
let _p = pushd(workspace::root_path()?)?; | |||
let sh = sh(); | |||
let _p = sh.push_dir(workspace::root_path()?); |
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.
I am not sure if I understand the initial intent here. Because pushd
/ Shell::push_dir
returns a RAII guard, it stop having an effect as soon as it is out of scope. And it is out of scope right after being created.
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.
The intention was to not change the directory to the workspace if the Changelog
command is used.
A let _p;
before the if branch give the guard the correct lifetime. Could you add that please?
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.
Done.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4358 +/- ##
==========================================
+ Coverage 85.09% 85.11% +0.01%
==========================================
Files 280 280
Lines 30535 30535
==========================================
+ Hits 25984 25990 +6
+ Misses 4551 4545 -6 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Kévin Commaille <[email protected]>
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.
Seems to work as it worked previously, the code looks sane as well.
Thanks for taking care of this.
Gets rid of an
unexpected_cfgs
warning. Alternative to #4352.Because most subcommands are functions, this uses a thread-local static shared shell API. If they were methods, we could just use a shared shell API for a given command.