Conversation
Codecov Report
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
- Coverage 95.32% 94.87% -0.46%
==========================================
Files 33 34 +1
Lines 1561 1599 +38
Branches 238 248 +10
==========================================
+ Hits 1488 1517 +29
- Misses 41 44 +3
- Partials 32 38 +6
Continue to review full report at Codecov.
|
|
This pull request introduces 1 alert and fixes 8 when merging d4db04d into b7f1f6c - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 3 alerts and fixes 9 when merging 776ea7d into b7f1f6c - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 3 alerts and fixes 9 when merging ac92b89 into b7f1f6c - view on LGTM.com new alerts:
fixed alerts:
|
| def __init__(self, rules: Dict[str, str]): | ||
| if "regex" in rules: | ||
| self.regex = re.compile(rules["regex"]) | ||
| self.regex = re.compile(rules["regex"]) if rules["regex"] else None |
There was a problem hiding this comment.
Spaghetti code, first you check if "regex" is in rules and then again you check if regex is in rules.
Would make more sense to write
if "regex" in rules:
self.regex = re.compile(rules["regex"])
else:
self.regex = None
Oh, now that I think of it it's even incorrect as well as "regex" is impossible to not be set and it is possible to set rules["regex"] = False which would be silly.
There was a problem hiding this comment.
It's not actually, the check is for when the regex string is an empty string, which would erroneously match everything. And regex might not be set at all, in the future another rule might be used instead.
It could be replaced with rules.get("regex", None) or None though, which is cleaner. Edit: Nevermind, actually it can't since it needs to pass through re.compile.
|
This pull request introduces 3 alerts and fixes 9 when merging 95eed67 into b7f1f6c - view on LGTM.com new alerts:
fixed alerts:
|
Equivalent PR in aw-server-rust: ActivityWatch/aw-server-rust#67