-
Notifications
You must be signed in to change notification settings - Fork 990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Moment.__sub__ and generalize Moment.__add__ #3216
Conversation
- Also bump mypy version to fix an incompatibility with python 3.8 - Fix new mypy warnings: - Add explicit !r to format statements touching bytes - Rename `value_equality.py` to avoid name collision with the decorator it defines - Ignore false positives related to abstract base type bounds
@@ -509,7 +508,7 @@ def attempt_sync_with_master(pr: PullRequestDetails | |||
data = { | |||
'base': pr.branch_name, | |||
'head': master_sha, | |||
'commit_message': 'Update branch (automerge)'.format(pr.branch_name) | |||
'commit_message': 'Update branch (automerge)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original looks like it was accidentally missing a {}. Suggestion:
'commit_message': 'Update branch {pr.branch_name} (automerge)'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it was intentional. When I changed it to not mention the branch name I just forgot to remove the format call.
'Code: {}. Content: {}.'.format(response.status_code, | ||
response.content)) | ||
'Code: {}. Content: {!r}.'.format( | ||
response.status_code, response.content)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a format string?
(Applies throughout.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged. Wanted to keep the refactoring minimal.
- `Moment.__add__` now accepts any `OP_TREE` - `Moment.__sub__` takes an `OP_TREE` and returns a moment with those operations removed - If there is an operation-to-remove that was not present, an error is raised - Also bump mypy version to fix an incompatibility with python 3.8 and fix new warnings - Add explicit !r to format statements touching bytes - Rename `value_equality.py` to avoid name collision with the decorator it defines - Ignore false positives related to abstract base type bounds
Moment.__add__
now accepts anyOP_TREE
Moment.__sub__
takes anOP_TREE
and returns a moment with those operations removedvalue_equality.py
to avoid name collision with the decorator it defines