Page MenuHomePhabricator

rust: Add a log file rotation utility
ClosedPublic

Authored by SimonSapin on Feb 17 2021, 7:58 AM.

Details

Summary

This is ported to Rust from mercurial/loggingutil.py.

The "builder" pattern is used to make it visible at call sites what the two
numeric parameters mean. In Python they might simply by keyword arguments.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Feb 17 2021, 7:58 AM
Alphare added inline comments.
rust/hg-core/src/logging.rs
60

Should be >=, even though it's not really important.

61

Shouldn't it be (1..=self.max_files)?

SimonSapin marked 2 inline comments as done.Feb 19 2021, 3:18 AM
SimonSapin added inline comments.
rust/hg-core/src/logging.rs
60

Done.

61

It could be if we renamed {name}.{i - 1} to {name}.{i} instead of {name}.{i} to {name}.{i + 1}.

SimonSapin marked 2 inline comments as done.Feb 19 2021, 3:31 AM
SimonSapin updated this revision to Diff 25660.
Alphare accepted this revision.Feb 19 2021, 4:01 AM
Alphare added inline comments.
rust/hg-core/src/logging.rs
61

Right, doesn't really matter. I wasn't sure of the behavior of the range operator when doing n..n.

Alphare requested changes to this revision.Feb 19 2021, 5:41 AM

/!\ CI failed for this change after you last updated.

As a note to Simon: you should rely on Baymax to re-send your patches after the initial phabsend. Hopefully we can keep the CI happy and that works well. :)

This revision now requires changes to proceed.Feb 19 2021, 5:41 AM

Alright, let’s try this: https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/18143

What’s a good workflow when I also have additional patches to add on top of the stack?

Alphare accepted this revision.Feb 19 2021, 6:10 AM

Alright, let’s try this: https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/18143
What’s a good workflow when I also have additional patches to add on top of the stack?

Wait for the CI to finish, and phabsend your stack. No-ops will not trigger notifications. In an ideal world: don't use Phabricator... but that's still a ways out. ;)

This revision now requires review to proceed.Feb 19 2021, 6:10 AM
baymax updated this revision to Diff 25678.Feb 19 2021, 6:29 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.Feb 19 2021, 7:56 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.