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

Removes redundant booleans #299

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

gilnobrega
Copy link

After the if clause (shift >= 128) fails, it is ensured that shift is less than 128 when it goes through that part of the code, hence (128 > shift) is the same as (shift < 128) which is always true. Therefore it can be removed from both clauses.

After the if clause (shift >= 128) fails, it is ensured that shift is less than 128 when it reaches that part of the code, hence (128 > shift) is the same as (shift < 128) which is always true and can be removed from both clauses.
@arvidn
Copy link
Contributor

arvidn commented Jul 26, 2021

this change is correct and silences the LGTM issues. I looked at this months ago, and I have some issues with this change:

  1. I don't believe there are any tests covering this change
  2. There may be tests in the original repository that we copy-pasted this code from, but we're not running them
  3. We really should use the version from the original repo and contribute changes like these (and perhaps tests if there aren't any)

@lgtm-com
Copy link

lgtm-com bot commented Jul 26, 2021

This pull request fixes 2 alerts when merging 4fcd6c6 into 32c7afa - view on LGTM.com

fixed alerts:

  • 2 for Comparison result is always the same

@github-actions
Copy link

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants