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

Removed an unnecessary check in code which always evaluates to true #33710

Merged
merged 1 commit into from
Jan 21, 2023

Conversation

openrefactory
Copy link
Contributor

In file: deferred_revenue_and_expense.py, method: generate_report_data, a logical expression uses the identity operator. A new object is created inside the identity check operation and then used for matching identity. Since this is a distinct, new object, it will not have identity an match with anything else. As a result, the identity check will have a logical short circuit. Moreover in line 378, we already create an entry inside a list. So, the check for emptiness is unnecessary. Currently, the test expression in line 381 is passing all the time.

We removed that unnecessary test expression.

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Jan 18, 2023
@ruthra-kumar ruthra-kumar self-assigned this Jan 20, 2023
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #33710 (f3a1ae7) into develop (6b31c27) will increase coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head f3a1ae7 differs from pull request most recent head 49aed7f. Consider uploading reports for the commit 49aed7f to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #33710   +/-   ##
========================================
  Coverage    64.14%   64.14%           
========================================
  Files          819      819           
  Lines        59160    59159    -1     
========================================
  Hits         37946    37946           
+ Misses       21214    21213    -1     
Impacted Files Coverage Δ
...evenue_and_expense/deferred_revenue_and_expense.py 61.61% <0.00%> (+0.30%) ⬆️

@ruthra-kumar ruthra-kumar merged commit 4cbd63e into frappe:develop Jan 21, 2023
@ruthra-kumar
Copy link
Member

@Mergifyio backport version-14-hotfix version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Jan 21, 2023

backport version-14-hotfix version-13-hotfix

✅ Backports have been created

deepeshgarg007 pushed a commit that referenced this pull request Jan 21, 2023
…backport #33710) (#33763)

fix: removed an unnecessary check which always evaluates to true

(cherry picked from commit 49aed7f)

Co-authored-by: OpenRefactory, Inc <56681071+openrefactory@users.noreply.github.com>
deepeshgarg007 pushed a commit that referenced this pull request Jan 21, 2023
…33710)

fix: removed an unnecessary check which always evaluates to true
developmentforpeople pushed a commit to Ayuda-Efectiva/erpnext that referenced this pull request Jan 26, 2023
…rappe#33710)

fix: removed an unnecessary check which always evaluates to true
SaiFi0102 pushed a commit to ServerManagementERPNext/erpnext that referenced this pull request Feb 22, 2023
…rappe#33710)

fix: removed an unnecessary check which always evaluates to true
@barredterra barredterra mentioned this pull request Mar 7, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants