Closed
Conversation
It adds little value and isn't always packaged, so make it optional.
It has exactly one usage, and that one does some munging of the result. It's simpler to use for that one user to use Time::Piece (which is core as of 5.10).
It's a bit overkill to use athinfo on localhost.
For some reason, AlbireoTestUtils wants to read Triage::TestInfo's TEST_INFO hash, which isn't actually exported, and asserts that the relevant key is defined. So just export it.
autodie has been around since 5.10 and Fatal is deprecated in favor of it. One complication: :void, which makes the following imports not autodie if their return isn't checked, no longer exists. But it doesn't seem that we actually cared about this behavior.
We can finally delete permatest/printk hopefully? Whatever behavior we observed with /dev/kmsg way back in the day has aged out, as far as I can tell.
It's currently uninterruptable, making you wait till timeout. Very annoying.
At present, cleanup pkill can fail like this: 2025-03-30 06:16:40,646 DEBUG [106393] SystemUtils - _doSystem: running: pkill -QUIT '^udsCalculateSize$' 2025-03-30 06:16:40,661 DEBUG [106393] SystemUtils - runSystemCommand: command exited with status 1 2025-03-30 06:16:40,661 DEBUG [106393] SystemUtils - runSystemCommand: stderr: [ pkill: pattern that searches for process name longer than 15 characters will result in zero matches Try `pkill -f' option to match against the complete command line. ] Most of the other pkill invocations already have -f, so add -f here too.
Member
lorelei-sakai
left a comment
lorelei-sakai
left a comment
There was a problem hiding this comment.
Definitely some good idea in here. We won't be able to aceppt this series in this form, though, because the common repo is really read-only. (It's a mirror of the Perforce source.)
I'll definitely try to get some of these (and your other series) pulled into main and common internally, though, which should end up getting reflected here.
Member
lorelei-sakai
left a comment
lorelei-sakai
left a comment
There was a problem hiding this comment.
There's a pending change to pull some of these things into common, so they should be updated early next week. I've noted which ones in the individual commits.
Member
|
The changes we are accepting should now be reflect on main. Since we will not take the remaining suggestions right now, this can be closed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A grab bag of random fixups I stumbled across when trying to get a test setup going.
I have no idea what format for changes in this repo is.