Skip to content
This repository was archived by the owner on Dec 8, 2023. It is now read-only.

feat: use walrus operator instead of match and if#22

Merged
morgante merged 4 commits intogetgrit:mainfrom
vlopezferrando:match-if-to-walrus
Sep 23, 2023
Merged

feat: use walrus operator instead of match and if#22
morgante merged 4 commits intogetgrit:mainfrom
vlopezferrando:match-if-to-walrus

Conversation

@vlopezferrando
Copy link
Contributor

Use the walrus operator for snippets with the pattern:

# Convert this:
math = re.match(regex):
if match:
   ...

# To this:
if math := re.match(regex):
   ...

Limitations:

  • If the match function is imported with an alias (e.g. from re import match as m), it will not be transformed.
  • When re.match is used, we do not check that re comes from import re.

Comment on lines +7 to +16
```python
# Convert this:
math = re.match(regex):
if match:
...

# To this:
if math := re.match(regex):
...
```
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not have these sorts of snippets in descriptions. This is actually the exact point of the samples embedded in the markdown.

You can move this down below as the first sample for this pattern, and that will make it automatically tested.


```

## Transforms a log statement
Copy link
Contributor

Choose a reason for hiding this comment

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

It's often easier to look at individual before/after pairs. I'd consider spitting this into multiple samples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I've split all test cases.

},
}

// TODO: we should also check if `re` is imported
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I follow the difference here. Either way please resolve this todo and avoid commented out code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the comment. I was thinking of double-checking that re is an imported module and not something else, but I guess it's nitpicking.

@morgante morgante merged commit 4a19f80 into getgrit:main Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants