-
Notifications
You must be signed in to change notification settings - Fork 137
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 Sonarqube reported Code Smells #548
Comments
I've made a pull request solving this issue |
Thanks @hallelshohat for fixing the security hotspots. Now what is left is some code smells here that if anyone is interested, can go through and fix. |
I have gone through the list of issues and resolved a few as false positive after our discussions on #564 @maaverik @mradenovic If you get time, go through the codesmells and let me know if we need there are any more such false positives |
@daltonfury42 Are there anymore code smells or can this issue be closed? |
Hey @skShekhar. Sorry, missed your comment here. There are still a few small issues left, like the many TODOs we have. Let's leave this open so that we at least have a tracker that we can look at. |
Hey @daltonfury42 I'm new to open source and I wanted to tackle this issue. |
Hi @MogakaMo welcome. Here is a list of tiny tech debts that you can solve: https://sonarcloud.io/project/issues?id=SimplQ_simplQ-frontend&open=AXfKRRGH1JV0IME_FJy0&resolved=false After that, you can take up a bigger task like #601 #643 is an even bigger project would require you to first do the backend APIs also. |
ok thanks for the heads up @daltonfury42. I'll get back when i'm done |
Hello @MogakaMo I just saw the list of issues described in the list and would be happy to contribute. |
Closing this since it looks like this is all covered. |
We have accumulated some easy to fix security vulnerabilities over the last few months over here.
These are straightforward easy fixes, good for a new comer to take up as a good first issue.
Once they are fixed, we should see if a branch protection rule can be set to catch them pre-merge
The text was updated successfully, but these errors were encountered: