This repository has been archived by the owner on Jan 23, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature: refactor and test team comm #127
Feature: refactor and test team comm #127
Changes from 15 commits
e086aab
5d0d0bb
c134c75
8480c2d
c9698fd
7f29d05
80f40eb
42837fb
dcf2176
cbf04a2
472c66e
06c2432
bee6fb3
6901e4a
a5f3cf8
939f7a4
51ef3ad
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I agree with that. We should change the team data message. This needs some changes down stream in the behavior and robot filter, but it should be fine.
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.
I'll create an issue for it 👍
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.
What is checked with the
if
? Is it aNone
check? If so, I prefer the explicitis not None
.Also, earlier we checked for the existence of attributes using
if hasattr(message, "offensive_side"):
why is it different here?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.
Good question, I've also taken the
hasattr
andif
checks from the previous code. I'll double check what the reasoning behind it wasThere 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.
Okay 👍