-
Notifications
You must be signed in to change notification settings - Fork 225
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
Investigate Android sound code issues reported by Codacy #2570
Comments
No it isn't, but it is not a real issue since it will always be initialized before starting Sound. (And I'm still hoping to get my sound-redesign implemented, since it will solve a lot of these sound related issues ;=)) |
If this is simple, we might as well try to resolve it in 3.11.0. |
@ann0see I'm happy to pick this one up, unless you're planning to? |
Feel free to do so. It should be a quick fix. |
Where on Github do I look to see Codacity output - is this on jamulussoftware/jamulus:main? |
No. It's just enabled for my repo for security reasons: https://app.codacy.com/gh/ann0see/jamulus/dashboard |
Given anyone can enable Codacity against Jamulus, I don't understand how there is any security benefit in not running the tool on each commit and reporting the findings as part of the build? We use CodeQL in a similar manner - that also reports potential security flaws. |
Codacy is an external proprietary tool while CodeQL is something supported by GitHub. Every plug-in in the official repo is another potential attack vector. |
That doesn't answer my point. If anyone can use Codacity to expose flaws in Jamulus, should the Jamulus team not take on responsibility by having the reports delivered to the team as part of each build? Either there's a use for the tool, or there isn't, right? |
Yes. This is true. We can enable Codacy for the repo. We just didn't as it is a proprietary tool. |
Describe the bug
Codacy complains that the following variable is not used (and I don't see that this is false):
jamulus/android/sound.cpp
Line 150 in a9deb22
Also this warning:
I assume this is a false positive?
To Reproduce
See https://app.codacy.com/gh/ann0see/jamulus/file/68862695471/issues/source?bid=21822857&fileBranchId=21822857
Expected behavior
No Codacy warnings
Screenshots
See above
Operating system
Android
Version of Jamulus
Latest
Additional context
@j-santander @sthenos could you please have a look at these warnings too?
The text was updated successfully, but these errors were encountered: