drill
drill copied to clipboard
apache
DRILL-7856 Add lgtm badge to Drill and fix alerts
DRILL-7856: Add lgtm badge to Drill and fix alerts
Description
Added lgtm badges to README.
Documentation
n/a
Testing
n/a
@DanielT504, please note that the aim of the DRILL-7856 is also to fix most of the alerts found by the lgtm service.
Hi, I understand that fixing the alerts is the priority, but given as there are hundreds I am wondering if I can make a separate issue(s) for fixing these alerts, and get this request accepted in the meantime, then target specific alerts afterwards.
@vvysotskyi Honestly there are a lot of stuff there that I'd be down to fix. @mkusnir has shown interest as well. Would it be alright if we made a few large prs to try and fix all of the alerts?
@vvysotskyi Honestly there are a lot of stuff there that I'd be down to fix. @mkusnir has shown interest as well. Would it be alright if we made a few large prs to try and fix all of the alerts?
@DanielT504, If you and @mkusnir will agree to that, I'm ok with that approach. Let's do this. Can you create a master JIRA and then add sub tasks for the alerts as you go?
@cgivre Hey I've made the JIRA https://issues.apache.org/jira/projects/DRILL/issues/DRILL-7878?filter=updatedrecently, is there anything should I add to them?
I'm also ok with such an approach. But anyway, we can merge this PR only after those warnings are resolved, though in separate PRs.
@vvysotskyi yeah for sure. A big thing is that we want to make sure that what we're changing/surpressing isnt too major. So with each alert (or file w/ alerts) I have a small justification of why I surpressed/changed it. It'd be really cluttered if we put them all on 3 big prs. Would you prefer it if we had smaller prs, or just do as many as possible?
I prefer a fewer number of PRs if possible. Sometimes it is required to look through the commits history or commit annotations in the class to find a commit with some changes. Also, it is easier for the reviewer to review similar changes in a single PR, and it will be possible to point out additional places with similar issues.
LGTM is closing in Dec, 2022. https://github.blog/2022-08-15-the-next-step-for-lgtm-com-github-code-scanning/