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

fix: Flatten sourceMap folder detection #2919

Merged
merged 1 commit into from
May 12, 2020

Conversation

nschonni
Copy link
Contributor

CodeQL was flagging the second else if.
Looked at code and inlined the single use so it's not executed if
first condition is true.

@lgtm-com
Copy link

lgtm-com bot commented May 11, 2020

This pull request fixes 1 alert when merging 067ae70 into 0d6c3cc - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@nschonni
Copy link
Contributor Author

@xzyfer trying out CodeQL/LGTM.com, but this was the only thing it found right now. There are some things on the libsass side, but I'll open an issue over there

@xzyfer
Copy link
Contributor

xzyfer commented May 12, 2020

Is the same possible without inlining the condition? The variable name was useful in conveying the intent. I think that's valuable given the messy state of the sourcemapa logic.

@nschonni
Copy link
Contributor Author

It is, but then it's evaluating the directory aspect even in the "happy path". Alternately, I could move it inside the final else

CodeQL was flagging the second `else if`.
@nschonni nschonni force-pushed the lgtm-warning-unused-condition branch from 067ae70 to 889a2fc Compare May 12, 2020 03:25
@nschonni
Copy link
Contributor Author

OK, moved it into the inner else. Does that look better?

Copy link
Contributor

@xzyfer xzyfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@nschonni nschonni merged commit e1fc158 into sass:master May 12, 2020
@nschonni nschonni deleted the lgtm-warning-unused-condition branch May 12, 2020 04:19
Friendly-users referenced this pull request in Friendly-users/node-sass Jul 9, 2024
-----
It is inappropriate to include political and offensive content in public code repositories.

Public code repositories should be neutral spaces for collaboration and community, free from personal or political views that could alienate or discriminate against others. Political content, especially that which targets or disparages minority groups, can be harmful and divisive. It can make people feel unwelcome and unsafe, and it can create a hostile work environment.

Please refrain from adding such content to public code repositories.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants