Fix F541: replace f-strings with no placeholders in lightning tests#8929
Fix F541: replace f-strings with no placeholders in lightning tests#8929nikunjkumar05 wants to merge 1 commit intoElementsProject:masterfrom
Conversation
|
@rustyrussell @daywalker90 @Lagrang3 please review my PR |
|
You need to add "Changelog-None" to the PR or commit message. Otherwise looks good to me. |
|
ok sir |
|
@rustyrussell @daywalker90 @Lagrang3 |
|
@nikunjkumar05 thanks for your contribution. You haven't replaced all of the possible F541 (f-strings missing placeholders). There are a lot of different files that have this dev string. We will holding off reviewing further until the PR has all of the replacements required. |
|
Ok @madelinevibes thank you for review I will fix it shortly |
|
great that you've had a review but now @nikunjkumar05 you need to update so the PR passes CI. Thanks! |
|
Hi @nikunjkumar05, I am not approving the workflow to run just yet. Since the last commit is guaranteed to fail CI, running it now would just waste build minutes. Let's get all those fixes in first. |
|
Please rebase (not merge!) and squash everything into one clean commit when you're done. It's quite messy right now. Thanks! |
|
I think ready for review |
nikunjkumar05
force-pushed
the
fix-bugs
branch
3 times, most recently
from
a82f611 to
bf5239f
Compare
|
Hi @madelinevibes ma’am, I believe the work is ready for review. Please let me know your feedback whenever you get a chance. |
madelinevibes
added
the
Status::Ready for Review
|
@daywalker90 @madelinevibes any more modification needed please let me know ? |
|
@nikunjkumar05 can you re-base please? if possible, by the end of this week so we can add this to 26.06? |
Sure @madelinevibes |
|
Done |
daywalker90
left a comment
daywalker90
left a comment
There was a problem hiding this comment.
I also found one more f string that doesn't use a variable at tests/test_opening.py:2226:
f'result in a commitment_transaction without outputs'. Please fix that one as well.
After that run uv run make gen to check if your code changes the generated files (it should not!).
Also, the commit message needs a complete replacement. It should describe your overall changes made by this PR, not just the last step. Something like chore: degenerate f-strings with no placeholders to simple strings
| if _version_not_supported: | ||
| raise RuntimeError( | ||
| f'The grpc package installed is at version {GRPC_VERSION},' | ||
| + f' but the generated code in node_pb2_grpc.py depends on' |
There was a problem hiding this comment.
This is a generated file, don't touch it.
There was a problem hiding this comment.
It's still getting touched!
| #[serde(rename_all = "lowercase")] | ||
| pub enum Request {{ | ||
| """ | ||
| .replace("{{", "{").replace("}}", "}"), |
There was a problem hiding this comment.
This will some day shoot us in the foot. Please remove this and replace the double brackets with single ones in place.
There was a problem hiding this comment.
Not resolved, read again.
madelinevibes
removed
the
Status::Ready for Review
|
@nikunjkumar05 bringing your attention to Daywalker's review comments for resolution as soon as practicable so we can hopefully get this PR into 26.06! |
Sure I will commit it as soon as possible ( before Monday) |
nikunjkumar05
force-pushed
the
fix-bugs
branch
2 times, most recently
from
8b87ad5 to
7bcf954
Compare
|
Hi @madelinevibes @daywalker90 can you please review the changes ? |
| if _version_not_supported: | ||
| raise RuntimeError( | ||
| f'The grpc package installed is at version {GRPC_VERSION},' | ||
| + f' but the generated code in node_pb2_grpc.py depends on' |
There was a problem hiding this comment.
It's still getting touched!
| #[serde(rename_all = "lowercase")] | ||
| pub enum Request {{ | ||
| """ | ||
| .replace("{{", "{").replace("}}", "}"), |
There was a problem hiding this comment.
Not resolved, read again.
nikunjkumar05
force-pushed
the
fix-bugs
branch
3 times, most recently
from
5df390f to
39a3b67
Compare
5 participants
Changelog-None
About PR
This PR fixes flake8 F541 (f-strings missing placeholders) by replacing f-strings that contain no placeholders with plain strings in the lightning test files. These were lint errors only but reduce noise and avoid confusion.
Why
F541 indicates an f-string with no {} placeholders — likely a typo. Replacing with a plain string removes the lint error and clarifies intent.
How to Test
flake8 lightning/tests || true
Checklist