Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix issue regarding duplicate state setting, and adding helpers #41
Conversation
johnsca
added some commits
Dec 13, 2015
johnsca
reviewed
Dec 14, 2015
| @@ -470,7 +494,8 @@ def set_state(self, state): | ||
| 'relation': self.relation_name, | ||
| 'conversations': [], | ||
| }) | ||
| - value['conversations'].append(self.key) | ||
| + if self.key not in value['conversations']: |
mbruzek
reviewed
Dec 14, 2015
| + """ | ||
| + Toggle the given state for this conversation. | ||
| + | ||
| + If ``active`` is not given, the state will be flipped from its current |
mbruzek
Dec 14, 2015
Contributor
Flipped is a bit confusing here, I would explicitly call out the state will be removed or set.
|
This looks like a great fix and was happy to see docstrings included! The "flipped" concept was a bit tough to get until I looked at the code and realized that it was just set_state() or remove_state(). Someone reading the just the docstring might not understand what happens in the code. Just a minor wording concern should not prevent this fix from landing. +1 from me |
added a commit
that referenced
this pull request
Dec 14, 2015
mbruzek
merged commit 618fb90
into
juju-solutions:master
Dec 14, 2015
1 check passed
continuous-integration/travis-ci/pr
The Travis CI build passed
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
johnsca commentedDec 14, 2015
No description provided.