Skip to content
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

Remove all uses of assert in the codebase #2590

Merged
merged 2 commits into from Dec 10, 2016
Merged

Remove all uses of assert in the codebase #2590

merged 2 commits into from Dec 10, 2016

Conversation

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Dec 10, 2016

The assert statement can be disabled (python -O) and often does not
explain what went wrong. This replaces all uses of assert with an if
and a ValueError.

This needs a logic review, I had to invert conditions manually.

Signed-off-by: Chris Warrick kwpolska@gmail.com

The `assert` statement can be disabled (`python -O`) and often does not
explain what went wrong. This replaces all uses of `assert` with an `if`
and a `ValueError`.

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Copy link
Contributor

@felixfontein felixfontein left a comment

LGTM (I checked the logic, not the syntax, obviously...)

@@ -66,7 +66,8 @@ def _do_classification(self, site):
for lang in site.config['TRANSLATIONS'].keys():
# Extract classifications for this language
classifications[lang] = taxonomy.classify(post, lang)
assert taxonomy.more_than_one_classifications_per_post or len(classifications[lang]) <= 1
if not taxonomy.more_than_one_classifications_per_post and len(classifications[lang]) > 1:
raise ValueError("Too many {0} classifications for post {1}".format(taxonomy.classification_name, post.source_path)
Copy link
Contributor

@felixfontein felixfontein Dec 10, 2016

There's one ) missing at the end of the line.

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska merged commit 64762d7 into master Dec 10, 2016
5 checks passed
@Kwpolska Kwpolska deleted the remove-asserts branch Dec 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants