Skip to content

Conversation

@pevogam
Copy link
Contributor

@pevogam pevogam commented Apr 3, 2025

No description provided.

Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
@pevogam pevogam force-pushed the linux-ops-fstring branch from 9f8ca15 to e6f8eaa Compare April 3, 2025 03:13
@pevogam
Copy link
Contributor Author

pevogam commented Apr 3, 2025

@ldoktor Just a minor fix here before the release. @lmr If you are not around could we provide access to @ldoktor or someone else for an annual release?

and this can lead to false-positives when used inside tests.
"""
LOG.info("Replacing {pattern} with {replacement} in {filename}")
LOG.info("Replacing %s with %s in %s", pattern, replacement, filename)
Copy link
Contributor

Choose a reason for hiding this comment

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

Well f-strings are recommended these days so how about adding the f before the string to make it work like a proper f-string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, this is exactly what I did originally but aexpect has a linter that would fail if one uses fstrings with logging (not the recommended way according to some standards) and all other aexpect logging messages use this formatting (and the linter passes for them) so I settled for what is most compatible with the project code base.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't noticed. Thanks, then this is definitely better (to log variables in case of msg format error)

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@ldoktor ldoktor merged commit c47fdfe into avocado-framework:main Apr 4, 2025
3 checks passed
@pevogam pevogam deleted the linux-ops-fstring branch April 4, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants