You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We recently added in a Code Climate integration, and it complains about some issues. It's not a huge deal, but it would be nice to address these issues before they get more out of hand.
Ideally, I would like to see the migrations folder ignored by code climate. I only want our actual source code and tests to be inspected by this tool. Things that are build tools, build artifacts, etc should not be included in this scan.
After looking at Code Climates documentation I created a .yml to tell Code Climate to ignore folders and files we specify. I added the migrations folder.
I think we can certainly raise the limit for LOC, I think 250 is a bit of a stretch. Alternatively, it might make sense to break up routes.py and test_routes.py into smaller files, but I don't think they're too big right now, personally.
As for the code complexity issues, if those functions can be refactored, that would be nice, but again, I'm not too worried about it. I'm not even sure how to fix the "too many early returns" thing. Maybe using variables and returning the value at the end? I think that a long &&|| string would be much harder to grok than just "return False"...
How about this: if you want to tweak the yaml file to get rid of all of them, we can review the rules in the PR and make specific decisions point by point. If we want to actually fix the code in any case, we'll do that.
@rooshs481 A lot of the code climate issues are with the tests. Please break this into 2 PRs, one where we just address the app changes and another where we just address the test changes. Right now, I trust the tests to tell us that everything we promise in the docs is working properly and I believe that we can use that confidence to make changes to the app only.
When we start messing with the tests, I don't want any app changes because I want to only have to focus on ensuring the tests still give us the same confidence. Thanks for taking this on.
@avats-dev I believe that was a 404 because we switched the default branch from master to main but never updated the default branch in Code Climate. Thanks for letting me know, I made the switch and it appears to be working now. Looks like there's 59 issues identified, so it appears you have your work cut out for you 😆 Thanks for picking this up
@aaron-suarez yeah, now link works and I checked out the errors and I'll get started on this. I'm a bit busy with some personal work but I want to contribute, so I might take some time to get this done. Hope that's okay. Thanks.
Sure @king-11! Maybe mention which ones you're doing so @avats-dev can make sure that he's working on different ones. There were over 50 issues last I checked so there should be plenty to go around
Activity
Optimus-PrimaNocta commentedon Jan 2, 2019
After looking at Code Climates documentation I created a .yml to tell Code Climate to ignore folders and files we specify. I added the migrations folder.
aaron-junot commentedon Jan 8, 2019
Merged #86
apex-omontgomery commentedon Apr 19, 2019
What else is needed for this?
aaron-junot commentedon Apr 19, 2019
There are now 17 issues. Follow the link to find out more.

https://codeclimate.com/github/OperationCode/resources_api/issues
Optimus-PrimaNocta commentedon Apr 19, 2019
It doesn't look like any of those would break anything, just mostly complexity issues. Did we want to add the pages to the .yml to ignore them?
aaron-junot commentedon Apr 19, 2019
I think we can certainly raise the limit for LOC, I think 250 is a bit of a stretch. Alternatively, it might make sense to break up
routes.pyandtest_routes.pyinto smaller files, but I don't think they're too big right now, personally.As for the code complexity issues, if those functions can be refactored, that would be nice, but again, I'm not too worried about it. I'm not even sure how to fix the "too many early returns" thing. Maybe using variables and returning the value at the end? I think that a long
&&||string would be much harder to grok than just "return False"...How about this: if you want to tweak the yaml file to get rid of all of them, we can review the rules in the PR and make specific decisions point by point. If we want to actually fix the code in any case, we'll do that.
aaron-junot commentedon Jun 4, 2020
@rooshs481 A lot of the code climate issues are with the tests. Please break this into 2 PRs, one where we just address the app changes and another where we just address the test changes. Right now, I trust the tests to tell us that everything we promise in the docs is working properly and I believe that we can use that confidence to make changes to the app only.
When we start messing with the tests, I don't want any app changes because I want to only have to focus on ensuring the tests still give us the same confidence. Thanks for taking this on.
avats-dev commentedon Oct 11, 2020
@aaron-suarez can I work on this? Also, the link given above for code climate issues gives a 404.
aaron-junot commentedon Oct 11, 2020
@avats-dev I believe that was a 404 because we switched the default branch from
mastertomainbut never updated the default branch in Code Climate. Thanks for letting me know, I made the switch and it appears to be working now. Looks like there's 59 issues identified, so it appears you have your work cut out for you 😆 Thanks for picking this upavats-dev commentedon Oct 12, 2020
@aaron-suarez yeah, now link works and I checked out the errors and I'll get started on this. I'm a bit busy with some personal work but I want to contribute, so I might take some time to get this done. Hope that's okay. Thanks.
aaron-junot commentedon Oct 12, 2020
This has sat for a very long time, I would be shocked if someone else picked it up. So take your time my friend
king-11 commentedon Oct 22, 2020
@aaron-suarez can I work on some of these issues
aaron-junot commentedon Oct 22, 2020
Sure @king-11! Maybe mention which ones you're doing so @avats-dev can make sure that he's working on different ones. There were over 50 issues last I checked so there should be plenty to go around
king-11 commentedon Oct 23, 2020
@avats-dev I will be working on minor duplication Issues to make code DRY which I found after applying the filter do tell if you have worked on some
danielburn5 commentedon Dec 9, 2023
@avats-dev @king-11 Have you folks started work on any of these issues? I'd like to contribute but don't want to duplicate effort.