As a developer, I want to reduce code issues revealed through static analysis #276
Comments
|
Original Redmine Comment Here's the mile-high view from SQ as of commit:wres|e955b3533d16b2f8f68ee392d0b117f05541bdbd: !sq_top.png! Not sure what is wrong with the code coverage metric, perhaps sq and other tooling not communicating, but that is obviously wrong. |
|
Original Redmine Comment !sq_vulns.png! |
|
Original Redmine Comment !bugs.png! |
|
Original Redmine Comment !reliability.png! |
|
Original Redmine Comment !security.png! |
|
Original Redmine Comment Reminded again that sq is really a neat tool. Obviously, some of these will be debatable or false positives, but a lot won't be. |
|
Original Redmine Comment Some of them are probably urgent. |
|
Original Redmine Comment Approaching 100k sloc too, sheesh. |
|
Original Redmine Comment As an aside, this is probably something we should add to our jenkins build. |
|
Original Redmine Comment James wrote:
|
|
Original Redmine Comment The ticket for the capability on the VLab side: #66414 |
|
Original Redmine Comment Jesse wrote:
Thanks. Doesn't hurt to link, I suppose, so I did. |
|
Original Redmine Comment Hadn't intended to start addressing these, but I guess I did. Sunk a couple of hours in. Good task between tasks. !sq_top2.png! A lot of the security suggestions are false positives. |
|
Original Redmine Comment Progress in commit:wres|d93a710e22c7d5acfb335b4a91c49c4fe0a59c1c. Again, the sort of ticket that anyone can increment between tasks. |
|
Original Redmine Comment Pushing a few more against this one. |
|
Original Redmine Comment commit:wres|054bc754d66a2799049aaca16ff76d877fed1944 |
|
Original Redmine Comment In preparing for some work on #59791, first fixing a bajillion code smells in @wres-io@ that have been hanging around forever. The lack of attention to detail in parts of that code base is frustrating, to say the least. |
|
Original Redmine Comment I see a different spelling of "bankful" (bankfull) in different WRDS APIs. I wonder if this is a typo on our end (and, if so, a bug, since Jackson maps on names) or if the APIs do vary... |
|
Original Redmine Comment About to push a lot of small changes. Some of these involve changes to Jackson annotations to deserialize json or waterml into POJOs, so we should also do some testing of all WRDS services, including thresholds, before deploying as I'm not sure how good the unit test coverage is for some of these classes. |
|
Original Redmine Comment Painful number of fixes here. commit:wres|522ad907ec14da7e17e5995d5366818f610e5774 |
|
Original Redmine Comment You'll probably want to pull at your earliest convenience. |
|
Original Redmine Comment There are still quite a few code smells throughout the codebase, but stopping there for now. This is one that we'll keep returning to. Good practice is to avoid these smells by installing static analysis tools to identify them as you develop, but I appreciate that there are some restrictions on OWP developers that make their lives harder, so this may not always be easy/possible. |
|
Original Redmine Comment James, I'll try to deploy to staging later today so that I can do some early testing of WRDS services. Thanks for the heads up, Hank |
|
Original Redmine Comment James wrote:
Doing some initial testing using 20230306-ff9d924. The following tests all used an SA executed on nwcal-wres-ti01 (Docker wrapped JDK image) and forecast period 3/1/2023 - 3/5/2023:
So far so good, Hank |
|
Original Redmine Comment The last evaluation using using the connection failed due to a disconnection. It was probably just a network hiccup. It certainly appears that the WRES successfully communicated with WRDS, given it reached the point of data acquisition, but I would like to see the evaluation complete. I'll login and try again, this time running the evaluation in a @nohup@. Hank |
|
Original Redmine Comment The evaluation succeeded in 25m 37s. Good. For now, my testing is done. I'll leave the ticket on-hold as a reminder to repeat these tests when we deploy. Hank |
|
Original Redmine Comment Spotted a bunch of additional issues related to declaration helpers, which I'm going to resolve alongside #113677. |
|
Original Redmine Comment commit:wres|f03f8bd65cf2e86148374ea59af3d918f16678c0 |
|
Original Redmine Comment As noted in the @build.gradle@: Going to try and fix this because it results in an annoying duplication warning at build time in the IDE from these files being duplicated on the cp. |
|
Original Redmine Comment James wrote:
Substantially fixed in commit:wres|12488e4e3b7719c9793762f8e4472ce7bfcd9f1a, although that contained a minor platform-dependent path error that was fixed in commit:wres|b13a758a1fdb7c9afae8d7f021f41578d952288a. To the backlog again. |
|
Original Redmine Comment Before starting some major refactoring for #113677, taking an opportunity to rename/relocate some things that are potentially confusing to new developers. Not identified by static analysis, as such, but this ticket is close enough. |
|
Original Redmine Comment James wrote:
Some of the things I'm looking at are also connected to #59791 - will tag that ticket in the relevant commits too. |
|
Original Redmine Comment First thing I want to do is to place everything related to reading from WRDS in one place. We have (WRDS) readers for features, thresholds and time-series data and they are variously distributed throughout @wres-io@. Since we have a @wres.io.reading.wrds@, it makes sense to place everything related to reading from WRDS in that package. |
|
Original Redmine Comment commit:wres|e3db852fe7f3f7811902504152b1c86a222f41a3 |
|
Original Redmine Comment Done with this (for now) as of commit:wres|76c53c091e08eaa8fef480ea36e0eaa58a0a2141. |
Author Name: James (James)
Original Redmine Issue: 96440, https://vlab.noaa.gov/redmine/issues/96440
Original Date: 2021-09-20
Original Assignee: Evan
Given a static analysis of the codebase performed with SonarQube or similar
When I see a lot of code smells, bugs, vulnerabilities etc.
Then I want to review and eliminate some of 'em
Redmine related issue(s): 96459, 112924
The text was updated successfully, but these errors were encountered: