Code inspection
Locked
123456
123456
 
|
+1
On Fri, Oct 26, 2018 at 9:45 AM Nikolay Izhikov <[hidden email]> wrote: > Hello, Maxim. > > Great job! Thank you! > Igniters, let's add this suite to the Run All! > > It can help us improve code quality and check code style without human > eyes. > > > В Пт, 26/10/2018 в 00:47 +0300, Maxim Muzafarov пишет: > > Igniters, > > > > The new `Inspections: Core` suite [2] configured on TeamCity (pass > > successfully with - Inspections total: 0, errors: 0 ). > > The next rules are enabled for this suite: > > - `Missorted modifiers`; > > - `'size() == 0' replaceable with 'isEmpty()'`; > > - `Add missing @Override annotation`; > > - `Fix unused imports`; > > > > Let's incule it to the `Run::All` group on TC, so we will check these > rules > > automatically for each PR. > > Any objections? > > > > > > Talking about the details, > > > > - the issue [1] with adding an inspections configuration for TC have PA > > status; > > - the new configuration ignite_inspections_teamcity.xml added to PR; > > - four rules which are already fixed in the master branch enabled in > config; > > - the `Inspections:Core` suite configured to use the inspections > > configuration from the local branch; > > - the example `how to use inspections from the command line` added. > > > > > > Petr, Nikolay, > > > > Thank you for your support! > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9983 > > [2] > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=pull%2F5059%2Fhead&tab=buildTypeStatusDiv > > [3] > > > https://issues.apache.org/jira/browse/IGNITE-9983?focusedCommentId=16662323&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16662323 > > > > On Tue, 23 Oct 2018 at 19:16 Nikolay Izhikov <[hidden email]> > wrote: > > > > > Hello, Maxim. > > > > > > +1 from me. > > > > > > I vote to enable static inspections for the Ignite codebase. > > > Thank you for that contributions! > > > > > > В Вт, 23/10/2018 в 19:09 +0300, Maxim Muzafarov пишет: > > > > Igniters, > > > > > > > > I've fixed some issues according to the inspections.xml > configuration: > > > > - `Missorted modifiers`; > > > > - `'size() == 0' replaceable with 'isEmpty()'`; > > > > - `Add missing @Override annotation`; > > > > These one have `In progress` state: > > > > - `Fix unused imports`; > > > > - `Remove unnecessary @SuppressWarnings annotation`; > > > > > > > > The list of issues related to the current Code Inspections changes > can be > > > > found [1] > > > > with using label `inspections`. So, to move forward and not lose > current > > > > changes I > > > > propose to: > > > > - Create the new configuration idea\ignite_inspections_teamcity.xml > (I > > > > will file a new issue for it); > > > > - Tune `Inspections: Core` Suite to use this configuration profile > (It > > > > will run with each PR); > > > > - In the case with fixing a new inspection rule enable it this > > > > > > inspection > > > > configuration. > > > > > > > > This will allow us to move forward in small steps and at some point > of > > > > > > time > > > > in future we will switch > > > > this ignite_inspections_teamcity.xml with the > > > > default ignite_inspections.xml. > > > > > > > > Thoughts? > > > > Pert Ivanov, will you help to tune `Inspections: Core` suite? > > > > > > > > [1] > > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-9923?jql=project%20%3D%20Ignite%20AND%20labels%20%3D%20inspections > > > > > > > > On Sat, 25 Aug 2018 at 00:54 Dmitriy Pavlov <[hidden email]> > > > > > > wrote: > > > > > > > > > IntelliJ Idea shows missing @Override annotation on my > installation. > > > > > > Not > > > > > sure it comes from our inspection or not. > > > > > > > > > > Anyway, count on me. > > > > > > > > > > пт, 24 авг. 2018 г. в 9:25, Maxim Muzafarov <[hidden email]>: > > > > > > > > > > > Folks, > > > > > > > > > > > > I think we can make a small step further with Ignite Inspections. > > > > > > > > > > > > I've created these tickets [1], [2] for myself according to > > > > > > previously > > > > > > added > > > > > > `idea/ignite_inspections.xml` and I plan to complete them. > > > > > > > > > > > > Who will help me with review and merge? > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9311 - Add > missing > > > > > > @Override annotation > > > > > > [2] https://issues.apache.org/jira/browse/IGNITE-9312 - Remove > > > > > > > > > > unnecessary > > > > > > @SuppressWarnings annotation > > > > > > > > > > > > On Thu, 16 Aug 2018 at 19:53 Dmitriy Pavlov < > [hidden email]> > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi Pavel, > > > > > > > > > > > > > > Thank you for noticing and bringing it here. I've fixed TC > failure. > > > > > > > > > > > > > > Sincerely, > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > чт, 16 авг. 2018 г. в 0:10, Pavel Pereslegin <[hidden email] > >: > > > > > > > > > > > > > > > Hello Igniters. > > > > > > > > > > > > > > > > It seems that "idea/ignite_inspections.xml" should be > excluded > > > > > > from > > > > > > > > "check-licenses" maven profile, because "_Licenses Headers_" > > > > > > > > configuration always fails now [1] on TeamCity. > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_LicensesHeaders&tab=buildTypeHistoryList&branch_IgniteTests24Java8=%3Cdefault%3E > > > > > > > > ср, 15 авг. 2018 г. в 20:49, Dmitriy Pavlov < > > > > > > [hidden email]>: > > > > > > > > > > > > > > > > > > I've updated wiki page > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-C.CodeInspection > > > > > > > > > with > > > > > > > > > reference to settings.xml placement in the project. > > > > > > > > > > > > > > > > > > It is only advice, so I hope you don't mind having this > > > > > > reference. > > > > > > > > > > > > > > > > > > ср, 15 авг. 2018 г. в 16:45, Dmitriy Pavlov < > > > > > > [hidden email] > > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > > > > > > > Thank you for stepping in. I've committed the first > version > > > > > > here > > > > > > > > > > 'idea/ignite_inspections.xml'. We can move it to project > > > > > > default > > > > > > > > > > > > > > later > > > > > > > > when > > > > > > > > > > all inspection problems are fixed. > > > > > > > > > > Commit: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://git-wip-us.apache.org/repos/asf?p=ignite.git;a=commit;h=3e0f04edf7cc0aa1631fbd1b9af1e9b87b697eb1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > > > > > you can enable this profile using the following steps: > > > > > > > > > > Inspections > > > > > > > > > > (icon)->Configure inspections->(settings button)->Import > > > > > > > > > > > > > > > > Profile->select > > > > > > > > > > file and import. > > > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > > > вт, 14 авг. 2018 г. в 16:31, Maxim Muzafarov < > > > > > > [hidden email] > > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > Dmitry and other Igniters, > > > > > > > > > > > > > > > > > > > > > > Previously you has suggested to commit `Code > Inspections` > > > > > > into > > > > > > > > > > > > > > Ignite > > > > > > > > > > > codebase. > > > > > > > > > > > It makes sense for me. I think it's the easiest way to > > > > > > share > > > > > > > > > > this > > > > > > > > profile > > > > > > > > > > > among community > > > > > > > > > > > members and this inspection can be set as for the > project > > > > > > level. > > > > > > > > > > > So, I suggest: > > > > > > > > > > > > > > > > > > > > > > 1) According to Jetbrains documentation [1] the > inspection > > > > > > > > > > profile > > > > > > > > can be > > > > > > > > > > > placed to > > > > > > > > > > > `<project>/.idea/inspectionProfiles` with name > > > > > > > > > > > > `Project_Default.xml` > > > > > > > > > > > (hope most of us using IDEA for development) > > > > > > > > > > > This allows enable this profile automatically on per > > > > > > project > > > > > > > > > > level > > > > > > > and > > > > > > > > > > > will > > > > > > > > > > > simplify > > > > > > > > > > > development process according to rules accepted by our > > > > > > > > > > community. > > > > > > > > > > > > > > > > > > > > > > 2) I can file tickets and do some of them to fix > inspection > > > > > > > > > > > > failures > > > > > > > > which > > > > > > > > > > > Alexey mentioned > > > > > > > > > > > earlier. Hope other members of community will help me > with > > > > > > it. > > > > > > > > > > > > > > > > > > > > > > 3) I think `Inspections (Core)` TeamCity can be > triggered > > > > > > as > > > > > > > > > > > > nightly > > > > > > > > build > > > > > > > > > > > as it takes more > > > > > > > > > > > than 4 hours. Suppose, inspection build in each PR is > not > > > > > > the > > > > > > > > > > best > > > > > > > > way in > > > > > > > > > > > our case. New run > > > > > > > > > > > here [2]. > > > > > > > > > > > > > > > > > > > > > > 4) We can tune our MTCGA.Bot to notify members with new > > > > > > > > > > inspection > > > > > > > > > > > failures > > > > > > > > > > > added by them. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, I've taken Alexey's inspection file as an > example, > > > > > > I’ve > > > > > > > > > > > > > > checked > > > > > > > > what > > > > > > > > > > > we already discussed > > > > > > > > > > > previously (e.g. `Anonymous class can be converted to > > > > > > lambda` > > > > > > > > > > > > should > > > > > > > > be > > > > > > > > > > > disabled by default) > > > > > > > > > > > and added these additional rules to it: > > > > > > > > > > > - General | Line is longer than allowed by code style > > > > > > > > > > > - Java | Code maturity | Call to 'printStackTrace()' > > > > > > > > > > > - Java | Code style issues | Unnecessary 'null' check > > > > > > before > > > > > > > > > > > > > > > > 'equals()' > > > > > > > > > > > call > > > > > > > > > > > > > > > > > > > > > > If we decide to proceed I will attach this file to > JIRA. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > https://www.jetbrains.com/help/idea/code-inspection.html > > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3710%2Fhead > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 14 Aug 2018 at 16:19 Dmitriy Pavlov < > > > > > > > > > > > > [hidden email]> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Dmitriy Govoruknin, > > > > > > > > > > > > > > > > > > > > > > > > It seems there is a lack of volunteers to apply code > > > > > > > > > > inspections > > > > > > > to > > > > > > > > our > > > > > > > > > > > CI > > > > > > > > > > > > & patch submission process. Probably we could come > back > > > > > > to > > > > > > > > > > your > > > > > > > > > > > > initial idea about setting up inspection locally. > > > > > > > > > > > > > > > > > > > > > > > > Could you commit or share your IDEA inspection > settings? > > > > > > I > > > > > > > > > > could > > > > > > > > apply > > > > > > > > > > > it > > > > > > > > > > > > at least on my machine and remove odd warning types > one > > > > > > by > > > > > > > > > > one. > > > > > > > > What do > > > > > > > > > > > you > > > > > > > > > > > > think? > > > > > > > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > > > > > > > сб, 4 авг. 2018 г. в 1:22, Dmitriy Pavlov < > > > > > > > > > > > > [hidden email] > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > > > > Ideally, I should not asking for people to solve > > > > > > something > > > > > > > > > > for > > > > > > > me. > > > > > > > > > > > > > > > > > > > > > > > > > > I just want this idea did not disappear from our > radar. > > > > > > > > > > > > > > > > > > > > > > > > > > пт, 3 авг. 2018 г. в 23:47, Dmitriy Setrakyan < > > > > > > > > > > > > > > > > [hidden email] > > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 3, 2018 at 7:49 AM, Dmitriy Pavlov < > > > > > > > > > > > > > > > > > > > > > > [hidden email]> > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I understand it is not so Apache-way from my > side > > > > > > to ask > > > > > > > > > > > > > > > > > > > > > > volunteers to > > > > > > > > > > > > > > do > > > > > > > > > > > > > > > some things (instead of contributing it by > myself). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Dmitriy, I am not sure why you feel this is not > the > > > > > > Apache > > > > > > > > > > > > way. > > > > > > > > No > > > > > > > > > > > one > > > > > > > > > > > > can > > > > > > > > > > > > > > do everything themselves. You should absolutely > keep > > > > > > > > > > > > > > recruiting > > > > > > > > more > > > > > > > > > > > > > > volunteers from the community. > > > > > > > > > > > > > > > > > > > > > > > > > > > > D. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > -- > > > > > > > > > > > Maxim Muzafarov > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -- > > > > > > Maxim Muzafarov > > > > > > > |
|
In reply to this post by Mmuzaf
Added -> Run :: IntelliJ IDEA Inspections [1] to Run All
Disclaimer: currently, on branches with no inspections file there is default inspections file with all inspections turned off [1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_RunIntelliJIdeaInspections > On 26 Oct 2018, at 00:47, Maxim Muzafarov <[hidden email]> wrote: > > Igniters, > > The new `Inspections: Core` suite [2] configured on TeamCity (pass > successfully with - Inspections total: 0, errors: 0 ). > The next rules are enabled for this suite: > - `Missorted modifiers`; > - `'size() == 0' replaceable with 'isEmpty()'`; > - `Add missing @Override annotation`; > - `Fix unused imports`; > > Let's incule it to the `Run::All` group on TC, so we will check these rules > automatically for each PR. > Any objections? > > > Talking about the details, > > - the issue [1] with adding an inspections configuration for TC have PA > status; > - the new configuration ignite_inspections_teamcity.xml added to PR; > - four rules which are already fixed in the master branch enabled in config; > - the `Inspections:Core` suite configured to use the inspections > configuration from the local branch; > - the example `how to use inspections from the command line` added. > > > Petr, Nikolay, > > Thank you for your support! > > [1] https://issues.apache.org/jira/browse/IGNITE-9983 > [2] > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=pull%2F5059%2Fhead&tab=buildTypeStatusDiv > [3] > https://issues.apache.org/jira/browse/IGNITE-9983?focusedCommentId=16662323&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16662323 > > On Tue, 23 Oct 2018 at 19:16 Nikolay Izhikov <[hidden email]> wrote: > >> Hello, Maxim. >> >> +1 from me. >> >> I vote to enable static inspections for the Ignite codebase. >> Thank you for that contributions! >> >> В Вт, 23/10/2018 в 19:09 +0300, Maxim Muzafarov пишет: >>> Igniters, >>> >>> I've fixed some issues according to the inspections.xml configuration: >>> - `Missorted modifiers`; >>> - `'size() == 0' replaceable with 'isEmpty()'`; >>> - `Add missing @Override annotation`; >>> These one have `In progress` state: >>> - `Fix unused imports`; >>> - `Remove unnecessary @SuppressWarnings annotation`; >>> >>> The list of issues related to the current Code Inspections changes can be >>> found [1] >>> with using label `inspections`. So, to move forward and not lose current >>> changes I >>> propose to: >>> - Create the new configuration idea\ignite_inspections_teamcity.xml (I >>> will file a new issue for it); >>> - Tune `Inspections: Core` Suite to use this configuration profile (It >>> will run with each PR); >>> - In the case with fixing a new inspection rule enable it this >> inspection >>> configuration. >>> >>> This will allow us to move forward in small steps and at some point of >> time >>> in future we will switch >>> this ignite_inspections_teamcity.xml with the >>> default ignite_inspections.xml. >>> >>> Thoughts? >>> Pert Ivanov, will you help to tune `Inspections: Core` suite? >>> >>> [1] >>> >> https://issues.apache.org/jira/browse/IGNITE-9923?jql=project%20%3D%20Ignite%20AND%20labels%20%3D%20inspections >>> >>> On Sat, 25 Aug 2018 at 00:54 Dmitriy Pavlov <[hidden email]> >> wrote: >>> >>>> IntelliJ Idea shows missing @Override annotation on my installation. >> Not >>>> sure it comes from our inspection or not. >>>> >>>> Anyway, count on me. >>>> >>>> пт, 24 авг. 2018 г. в 9:25, Maxim Muzafarov <[hidden email]>: >>>> >>>>> Folks, >>>>> >>>>> I think we can make a small step further with Ignite Inspections. >>>>> >>>>> I've created these tickets [1], [2] for myself according to >> previously >>>>> added >>>>> `idea/ignite_inspections.xml` and I plan to complete them. >>>>> >>>>> Who will help me with review and merge? >>>>> >>>>> [1] https://issues.apache.org/jira/browse/IGNITE-9311 - Add missing >>>>> @Override annotation >>>>> [2] https://issues.apache.org/jira/browse/IGNITE-9312 - Remove >>>> >>>> unnecessary >>>>> @SuppressWarnings annotation >>>>> >>>>> On Thu, 16 Aug 2018 at 19:53 Dmitriy Pavlov <[hidden email]> >>>> >>>> wrote: >>>>> >>>>>> Hi Pavel, >>>>>> >>>>>> Thank you for noticing and bringing it here. I've fixed TC failure. >>>>>> >>>>>> Sincerely, >>>>>> Dmitriy Pavlov >>>>>> >>>>>> чт, 16 авг. 2018 г. в 0:10, Pavel Pereslegin <[hidden email]>: >>>>>> >>>>>>> Hello Igniters. >>>>>>> >>>>>>> It seems that "idea/ignite_inspections.xml" should be excluded >> from >>>>>>> "check-licenses" maven profile, because "_Licenses Headers_" >>>>>>> configuration always fails now [1] on TeamCity. >>>>>>> >>>>>>> [1] >>>>>>> >>>> >>>> >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_LicensesHeaders&tab=buildTypeHistoryList&branch_IgniteTests24Java8=%3Cdefault%3E >>>>>>> ср, 15 авг. 2018 г. в 20:49, Dmitriy Pavlov < >> [hidden email]>: >>>>>>>> >>>>>>>> I've updated wiki page >>>>>>>> >>>> >>>> >> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-C.CodeInspection >>>>>>>> with >>>>>>>> reference to settings.xml placement in the project. >>>>>>>> >>>>>>>> It is only advice, so I hope you don't mind having this >> reference. >>>>>>>> >>>>>>>> ср, 15 авг. 2018 г. в 16:45, Dmitriy Pavlov < >> [hidden email] >>>>> >>>>> : >>>>>>>> >>>>>>>>> Hi Maxim, >>>>>>>>> >>>>>>>>> Thank you for stepping in. I've committed the first version >> here >>>>>>>>> 'idea/ignite_inspections.xml'. We can move it to project >> default >>>>>> >>>>>> later >>>>>>> when >>>>>>>>> all inspection problems are fixed. >>>>>>>>> Commit: >>>>>>>>> >>>>>>>>> >>>> >>>> >> https://git-wip-us.apache.org/repos/asf?p=ignite.git;a=commit;h=3e0f04edf7cc0aa1631fbd1b9af1e9b87b697eb1 >>>>>>>>> >>>>>>>>> >>>>>>>>> Igniters, >>>>>>>>> >>>>>>>>> you can enable this profile using the following steps: >>>> >>>> Inspections >>>>>>>>> (icon)->Configure inspections->(settings button)->Import >>>>>>> >>>>>>> Profile->select >>>>>>>>> file and import. >>>>>>>>> >>>>>>>>> Sincerely, >>>>>>>>> Dmitriy Pavlov >>>>>>>>> >>>>>>>>> вт, 14 авг. 2018 г. в 16:31, Maxim Muzafarov < >> [hidden email] >>>>> >>>>> : >>>>>>>>> >>>>>>>>>> Dmitry and other Igniters, >>>>>>>>>> >>>>>>>>>> Previously you has suggested to commit `Code Inspections` >> into >>>>>> >>>>>> Ignite >>>>>>>>>> codebase. >>>>>>>>>> It makes sense for me. I think it's the easiest way to >> share >>>> >>>> this >>>>>>> profile >>>>>>>>>> among community >>>>>>>>>> members and this inspection can be set as for the project >> level. >>>>>>>>>> So, I suggest: >>>>>>>>>> >>>>>>>>>> 1) According to Jetbrains documentation [1] the inspection >>>> >>>> profile >>>>>>> can be >>>>>>>>>> placed to >>>>>>>>>> `<project>/.idea/inspectionProfiles` with name >>>>> >>>>> `Project_Default.xml` >>>>>>>>>> (hope most of us using IDEA for development) >>>>>>>>>> This allows enable this profile automatically on per >> project >>>> >>>> level >>>>>> and >>>>>>>>>> will >>>>>>>>>> simplify >>>>>>>>>> development process according to rules accepted by our >>>> >>>> community. >>>>>>>>>> >>>>>>>>>> 2) I can file tickets and do some of them to fix inspection >>>>> >>>>> failures >>>>>>> which >>>>>>>>>> Alexey mentioned >>>>>>>>>> earlier. Hope other members of community will help me with >> it. >>>>>>>>>> >>>>>>>>>> 3) I think `Inspections (Core)` TeamCity can be triggered >> as >>>>> >>>>> nightly >>>>>>> build >>>>>>>>>> as it takes more >>>>>>>>>> than 4 hours. Suppose, inspection build in each PR is not >> the >>>> >>>> best >>>>>>> way in >>>>>>>>>> our case. New run >>>>>>>>>> here [2]. >>>>>>>>>> >>>>>>>>>> 4) We can tune our MTCGA.Bot to notify members with new >>>> >>>> inspection >>>>>>>>>> failures >>>>>>>>>> added by them. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Also, I've taken Alexey's inspection file as an example, >> I’ve >>>>>> >>>>>> checked >>>>>>> what >>>>>>>>>> we already discussed >>>>>>>>>> previously (e.g. `Anonymous class can be converted to >> lambda` >>>>> >>>>> should >>>>>>> be >>>>>>>>>> disabled by default) >>>>>>>>>> and added these additional rules to it: >>>>>>>>>> - General | Line is longer than allowed by code style >>>>>>>>>> - Java | Code maturity | Call to 'printStackTrace()' >>>>>>>>>> - Java | Code style issues | Unnecessary 'null' check >> before >>>>>>> >>>>>>> 'equals()' >>>>>>>>>> call >>>>>>>>>> >>>>>>>>>> If we decide to proceed I will attach this file to JIRA. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> [1] >> https://www.jetbrains.com/help/idea/code-inspection.html >>>>>>>>>> [2] >>>>>>>>>> >>>>>>>>>> >>>> >>>> >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3710%2Fhead >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Tue, 14 Aug 2018 at 16:19 Dmitriy Pavlov < >>>>> >>>>> [hidden email]> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Dmitriy Govoruknin, >>>>>>>>>>> >>>>>>>>>>> It seems there is a lack of volunteers to apply code >>>> >>>> inspections >>>>>> to >>>>>>> our >>>>>>>>>> CI >>>>>>>>>>> & patch submission process. Probably we could come back >> to >>>> >>>> your >>>>>>>>>>> initial idea about setting up inspection locally. >>>>>>>>>>> >>>>>>>>>>> Could you commit or share your IDEA inspection settings? >> I >>>> >>>> could >>>>>>> apply >>>>>>>>>> it >>>>>>>>>>> at least on my machine and remove odd warning types one >> by >>>> >>>> one. >>>>>>> What do >>>>>>>>>> you >>>>>>>>>>> think? >>>>>>>>>>> >>>>>>>>>>> Sincerely, >>>>>>>>>>> Dmitriy Pavlov >>>>>>>>>>> >>>>>>>>>>> сб, 4 авг. 2018 г. в 1:22, Dmitriy Pavlov < >>>>> >>>>> [hidden email] >>>>>>> : >>>>>>>>>>> >>>>>>>>>>>> Ideally, I should not asking for people to solve >> something >>>> >>>> for >>>>>> me. >>>>>>>>>>>> >>>>>>>>>>>> I just want this idea did not disappear from our radar. >>>>>>>>>>>> >>>>>>>>>>>> пт, 3 авг. 2018 г. в 23:47, Dmitriy Setrakyan < >>>>>>> >>>>>>> [hidden email] >>>>>>>>>>> : >>>>>>>>>>>> >>>>>>>>>>>>> On Fri, Aug 3, 2018 at 7:49 AM, Dmitriy Pavlov < >>>>>>>>>> >>>>>>>>>> [hidden email]> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> I understand it is not so Apache-way from my side >> to ask >>>>>>>>>> >>>>>>>>>> volunteers to >>>>>>>>>>>>> do >>>>>>>>>>>>>> some things (instead of contributing it by myself). >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Dmitriy, I am not sure why you feel this is not the >> Apache >>>>> >>>>> way. >>>>>>> No >>>>>>>>>> one >>>>>>>>>>> can >>>>>>>>>>>>> do everything themselves. You should absolutely keep >>>>>> >>>>>> recruiting >>>>>>> more >>>>>>>>>>>>> volunteers from the community. >>>>>>>>>>>>> >>>>>>>>>>>>> D. >>>>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> -- >>>>>>>>>> Maxim Muzafarov >>>>>>>>>> >>>>> >>>>> -- >>>>> -- >>>>> Maxim Muzafarov >>>>> >> > -- > -- > Maxim Muzafarov |
|
|
In reply to this post by Mmuzaf
Hello, Maxim!
Good job! пт, 26 окт. 2018 г. в 0:47, Maxim Muzafarov <[hidden email]>: > Igniters, > > The new `Inspections: Core` suite [2] configured on TeamCity (pass > successfully with - Inspections total: 0, errors: 0 ). > The next rules are enabled for this suite: > - `Missorted modifiers`; > - `'size() == 0' replaceable with 'isEmpty()'`; > - `Add missing @Override annotation`; > - `Fix unused imports`; > > Let's incule it to the `Run::All` group on TC, so we will check these rules > automatically for each PR. > Any objections? > > > Talking about the details, > > - the issue [1] with adding an inspections configuration for TC have PA > status; > - the new configuration ignite_inspections_teamcity.xml added to PR; > - four rules which are already fixed in the master branch enabled in > config; > - the `Inspections:Core` suite configured to use the inspections > configuration from the local branch; > - the example `how to use inspections from the command line` added. > > > Petr, Nikolay, > > Thank you for your support! > > [1] https://issues.apache.org/jira/browse/IGNITE-9983 > [2] > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=pull%2F5059%2Fhead&tab=buildTypeStatusDiv > [3] > > https://issues.apache.org/jira/browse/IGNITE-9983?focusedCommentId=16662323&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16662323 > > On Tue, 23 Oct 2018 at 19:16 Nikolay Izhikov <[hidden email]> wrote: > > > Hello, Maxim. > > > > +1 from me. > > > > I vote to enable static inspections for the Ignite codebase. > > Thank you for that contributions! > > > > В Вт, 23/10/2018 в 19:09 +0300, Maxim Muzafarov пишет: > > > Igniters, > > > > > > I've fixed some issues according to the inspections.xml configuration: > > > - `Missorted modifiers`; > > > - `'size() == 0' replaceable with 'isEmpty()'`; > > > - `Add missing @Override annotation`; > > > These one have `In progress` state: > > > - `Fix unused imports`; > > > - `Remove unnecessary @SuppressWarnings annotation`; > > > > > > The list of issues related to the current Code Inspections changes can > be > > > found [1] > > > with using label `inspections`. So, to move forward and not lose > current > > > changes I > > > propose to: > > > - Create the new configuration idea\ignite_inspections_teamcity.xml (I > > > will file a new issue for it); > > > - Tune `Inspections: Core` Suite to use this configuration profile (It > > > will run with each PR); > > > - In the case with fixing a new inspection rule enable it this > > inspection > > > configuration. > > > > > > This will allow us to move forward in small steps and at some point of > > time > > > in future we will switch > > > this ignite_inspections_teamcity.xml with the > > > default ignite_inspections.xml. > > > > > > Thoughts? > > > Pert Ivanov, will you help to tune `Inspections: Core` suite? > > > > > > [1] > > > > > > https://issues.apache.org/jira/browse/IGNITE-9923?jql=project%20%3D%20Ignite%20AND%20labels%20%3D%20inspections > > > > > > On Sat, 25 Aug 2018 at 00:54 Dmitriy Pavlov <[hidden email]> > > wrote: > > > > > > > IntelliJ Idea shows missing @Override annotation on my installation. > > Not > > > > sure it comes from our inspection or not. > > > > > > > > Anyway, count on me. > > > > > > > > пт, 24 авг. 2018 г. в 9:25, Maxim Muzafarov <[hidden email]>: > > > > > > > > > Folks, > > > > > > > > > > I think we can make a small step further with Ignite Inspections. > > > > > > > > > > I've created these tickets [1], [2] for myself according to > > previously > > > > > added > > > > > `idea/ignite_inspections.xml` and I plan to complete them. > > > > > > > > > > Who will help me with review and merge? > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9311 - Add > missing > > > > > @Override annotation > > > > > [2] https://issues.apache.org/jira/browse/IGNITE-9312 - Remove > > > > > > > > unnecessary > > > > > @SuppressWarnings annotation > > > > > > > > > > On Thu, 16 Aug 2018 at 19:53 Dmitriy Pavlov <[hidden email] > > > > > > > > > > wrote: > > > > > > > > > > > Hi Pavel, > > > > > > > > > > > > Thank you for noticing and bringing it here. I've fixed TC > failure. > > > > > > > > > > > > Sincerely, > > > > > > Dmitriy Pavlov > > > > > > > > > > > > чт, 16 авг. 2018 г. в 0:10, Pavel Pereslegin <[hidden email]>: > > > > > > > > > > > > > Hello Igniters. > > > > > > > > > > > > > > It seems that "idea/ignite_inspections.xml" should be excluded > > from > > > > > > > "check-licenses" maven profile, because "_Licenses Headers_" > > > > > > > configuration always fails now [1] on TeamCity. > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_LicensesHeaders&tab=buildTypeHistoryList&branch_IgniteTests24Java8=%3Cdefault%3E > > > > > > > ср, 15 авг. 2018 г. в 20:49, Dmitriy Pavlov < > > [hidden email]>: > > > > > > > > > > > > > > > > I've updated wiki page > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-C.CodeInspection > > > > > > > > with > > > > > > > > reference to settings.xml placement in the project. > > > > > > > > > > > > > > > > It is only advice, so I hope you don't mind having this > > reference. > > > > > > > > > > > > > > > > ср, 15 авг. 2018 г. в 16:45, Dmitriy Pavlov < > > [hidden email] > > > > > > > > > > : > > > > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > > > > > Thank you for stepping in. I've committed the first version > > here > > > > > > > > > 'idea/ignite_inspections.xml'. We can move it to project > > default > > > > > > > > > > > > later > > > > > > > when > > > > > > > > > all inspection problems are fixed. > > > > > > > > > Commit: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://git-wip-us.apache.org/repos/asf?p=ignite.git;a=commit;h=3e0f04edf7cc0aa1631fbd1b9af1e9b87b697eb1 > > > > > > > > > > > > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > > > you can enable this profile using the following steps: > > > > > > > > Inspections > > > > > > > > > (icon)->Configure inspections->(settings button)->Import > > > > > > > > > > > > > > Profile->select > > > > > > > > > file and import. > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > вт, 14 авг. 2018 г. в 16:31, Maxim Muzafarov < > > [hidden email] > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > Dmitry and other Igniters, > > > > > > > > > > > > > > > > > > > > Previously you has suggested to commit `Code Inspections` > > into > > > > > > > > > > > > Ignite > > > > > > > > > > codebase. > > > > > > > > > > It makes sense for me. I think it's the easiest way to > > share > > > > > > > > this > > > > > > > profile > > > > > > > > > > among community > > > > > > > > > > members and this inspection can be set as for the project > > level. > > > > > > > > > > So, I suggest: > > > > > > > > > > > > > > > > > > > > 1) According to Jetbrains documentation [1] the > inspection > > > > > > > > profile > > > > > > > can be > > > > > > > > > > placed to > > > > > > > > > > `<project>/.idea/inspectionProfiles` with name > > > > > > > > > > `Project_Default.xml` > > > > > > > > > > (hope most of us using IDEA for development) > > > > > > > > > > This allows enable this profile automatically on per > > project > > > > > > > > level > > > > > > and > > > > > > > > > > will > > > > > > > > > > simplify > > > > > > > > > > development process according to rules accepted by our > > > > > > > > community. > > > > > > > > > > > > > > > > > > > > 2) I can file tickets and do some of them to fix > inspection > > > > > > > > > > failures > > > > > > > which > > > > > > > > > > Alexey mentioned > > > > > > > > > > earlier. Hope other members of community will help me > with > > it. > > > > > > > > > > > > > > > > > > > > 3) I think `Inspections (Core)` TeamCity can be triggered > > as > > > > > > > > > > nightly > > > > > > > build > > > > > > > > > > as it takes more > > > > > > > > > > than 4 hours. Suppose, inspection build in each PR is not > > the > > > > > > > > best > > > > > > > way in > > > > > > > > > > our case. New run > > > > > > > > > > here [2]. > > > > > > > > > > > > > > > > > > > > 4) We can tune our MTCGA.Bot to notify members with new > > > > > > > > inspection > > > > > > > > > > failures > > > > > > > > > > added by them. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, I've taken Alexey's inspection file as an example, > > I’ve > > > > > > > > > > > > checked > > > > > > > what > > > > > > > > > > we already discussed > > > > > > > > > > previously (e.g. `Anonymous class can be converted to > > lambda` > > > > > > > > > > should > > > > > > > be > > > > > > > > > > disabled by default) > > > > > > > > > > and added these additional rules to it: > > > > > > > > > > - General | Line is longer than allowed by code style > > > > > > > > > > - Java | Code maturity | Call to 'printStackTrace()' > > > > > > > > > > - Java | Code style issues | Unnecessary 'null' check > > before > > > > > > > > > > > > > > 'equals()' > > > > > > > > > > call > > > > > > > > > > > > > > > > > > > > If we decide to proceed I will attach this file to JIRA. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > https://www.jetbrains.com/help/idea/code-inspection.html > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3710%2Fhead > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 14 Aug 2018 at 16:19 Dmitriy Pavlov < > > > > > > > > > > [hidden email]> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hi Dmitriy Govoruknin, > > > > > > > > > > > > > > > > > > > > > > It seems there is a lack of volunteers to apply code > > > > > > > > inspections > > > > > > to > > > > > > > our > > > > > > > > > > CI > > > > > > > > > > > & patch submission process. Probably we could come back > > to > > > > > > > > your > > > > > > > > > > > initial idea about setting up inspection locally. > > > > > > > > > > > > > > > > > > > > > > Could you commit or share your IDEA inspection > settings? > > I > > > > > > > > could > > > > > > > apply > > > > > > > > > > it > > > > > > > > > > > at least on my machine and remove odd warning types one > > by > > > > > > > > one. > > > > > > > What do > > > > > > > > > > you > > > > > > > > > > > think? > > > > > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > > > > > сб, 4 авг. 2018 г. в 1:22, Dmitriy Pavlov < > > > > > > > > > > [hidden email] > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > > Ideally, I should not asking for people to solve > > something > > > > > > > > for > > > > > > me. > > > > > > > > > > > > > > > > > > > > > > > > I just want this idea did not disappear from our > radar. > > > > > > > > > > > > > > > > > > > > > > > > пт, 3 авг. 2018 г. в 23:47, Dmitriy Setrakyan < > > > > > > > > > > > > > > [hidden email] > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 3, 2018 at 7:49 AM, Dmitriy Pavlov < > > > > > > > > > > > > > > > > > > > > [hidden email]> > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I understand it is not so Apache-way from my side > > to ask > > > > > > > > > > > > > > > > > > > > volunteers to > > > > > > > > > > > > > do > > > > > > > > > > > > > > some things (instead of contributing it by > myself). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Dmitriy, I am not sure why you feel this is not the > > Apache > > > > > > > > > > way. > > > > > > > No > > > > > > > > > > one > > > > > > > > > > > can > > > > > > > > > > > > > do everything themselves. You should absolutely > keep > > > > > > > > > > > > recruiting > > > > > > > more > > > > > > > > > > > > > volunteers from the community. > > > > > > > > > > > > > > > > > > > > > > > > > > D. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > -- > > > > > > > > > > Maxim Muzafarov > > > > > > > > > > > > > > > > > > > > -- > > > > > -- > > > > > Maxim Muzafarov > > > > > > > > -- > -- > Maxim Muzafarov > -- BR, Sergey Antonov |
|
Petr,
> Disclaimer: currently, on branches with no inspections file there is default inspections file with all inspections turned off I'd suggest another approach: - we should use the default inspection configuration file only for old release branches like (2.5, 2.6 etc.); - the other branches should merge the latest changes from the master branch and fix all local issues according to configured inspection rules; The other situations may lead us to new inspection issues come from merged obsolete PRs. On Fri, 26 Oct 2018 at 11:53 Sergey Antonov <[hidden email]> wrote: > Hello, Maxim! > > Good job! > > пт, 26 окт. 2018 г. в 0:47, Maxim Muzafarov <[hidden email]>: > > > Igniters, > > > > The new `Inspections: Core` suite [2] configured on TeamCity (pass > > successfully with - Inspections total: 0, errors: 0 ). > > The next rules are enabled for this suite: > > - `Missorted modifiers`; > > - `'size() == 0' replaceable with 'isEmpty()'`; > > - `Add missing @Override annotation`; > > - `Fix unused imports`; > > > > Let's incule it to the `Run::All` group on TC, so we will check these > rules > > automatically for each PR. > > Any objections? > > > > > > Talking about the details, > > > > - the issue [1] with adding an inspections configuration for TC have PA > > status; > > - the new configuration ignite_inspections_teamcity.xml added to PR; > > - four rules which are already fixed in the master branch enabled in > > config; > > - the `Inspections:Core` suite configured to use the inspections > > configuration from the local branch; > > - the example `how to use inspections from the command line` added. > > > > > > Petr, Nikolay, > > > > Thank you for your support! > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9983 > > [2] > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=pull%2F5059%2Fhead&tab=buildTypeStatusDiv > > [3] > > > > > https://issues.apache.org/jira/browse/IGNITE-9983?focusedCommentId=16662323&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16662323 > > > > On Tue, 23 Oct 2018 at 19:16 Nikolay Izhikov <[hidden email]> > wrote: > > > > > Hello, Maxim. > > > > > > +1 from me. > > > > > > I vote to enable static inspections for the Ignite codebase. > > > Thank you for that contributions! > > > > > > В Вт, 23/10/2018 в 19:09 +0300, Maxim Muzafarov пишет: > > > > Igniters, > > > > > > > > I've fixed some issues according to the inspections.xml > configuration: > > > > - `Missorted modifiers`; > > > > - `'size() == 0' replaceable with 'isEmpty()'`; > > > > - `Add missing @Override annotation`; > > > > These one have `In progress` state: > > > > - `Fix unused imports`; > > > > - `Remove unnecessary @SuppressWarnings annotation`; > > > > > > > > The list of issues related to the current Code Inspections changes > can > > be > > > > found [1] > > > > with using label `inspections`. So, to move forward and not lose > > current > > > > changes I > > > > propose to: > > > > - Create the new configuration idea\ignite_inspections_teamcity.xml > (I > > > > will file a new issue for it); > > > > - Tune `Inspections: Core` Suite to use this configuration profile > (It > > > > will run with each PR); > > > > - In the case with fixing a new inspection rule enable it this > > > inspection > > > > configuration. > > > > > > > > This will allow us to move forward in small steps and at some point > of > > > time > > > > in future we will switch > > > > this ignite_inspections_teamcity.xml with the > > > > default ignite_inspections.xml. > > > > > > > > Thoughts? > > > > Pert Ivanov, will you help to tune `Inspections: Core` suite? > > > > > > > > [1] > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-9923?jql=project%20%3D%20Ignite%20AND%20labels%20%3D%20inspections > > > > > > > > On Sat, 25 Aug 2018 at 00:54 Dmitriy Pavlov <[hidden email]> > > > wrote: > > > > > > > > > IntelliJ Idea shows missing @Override annotation on my > installation. > > > Not > > > > > sure it comes from our inspection or not. > > > > > > > > > > Anyway, count on me. > > > > > > > > > > пт, 24 авг. 2018 г. в 9:25, Maxim Muzafarov <[hidden email]>: > > > > > > > > > > > Folks, > > > > > > > > > > > > I think we can make a small step further with Ignite Inspections. > > > > > > > > > > > > I've created these tickets [1], [2] for myself according to > > > previously > > > > > > added > > > > > > `idea/ignite_inspections.xml` and I plan to complete them. > > > > > > > > > > > > Who will help me with review and merge? > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9311 - Add > > missing > > > > > > @Override annotation > > > > > > [2] https://issues.apache.org/jira/browse/IGNITE-9312 - Remove > > > > > > > > > > unnecessary > > > > > > @SuppressWarnings annotation > > > > > > > > > > > > On Thu, 16 Aug 2018 at 19:53 Dmitriy Pavlov < > [hidden email] > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi Pavel, > > > > > > > > > > > > > > Thank you for noticing and bringing it here. I've fixed TC > > failure. > > > > > > > > > > > > > > Sincerely, > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > чт, 16 авг. 2018 г. в 0:10, Pavel Pereslegin <[hidden email] > >: > > > > > > > > > > > > > > > Hello Igniters. > > > > > > > > > > > > > > > > It seems that "idea/ignite_inspections.xml" should be > excluded > > > from > > > > > > > > "check-licenses" maven profile, because "_Licenses Headers_" > > > > > > > > configuration always fails now [1] on TeamCity. > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_LicensesHeaders&tab=buildTypeHistoryList&branch_IgniteTests24Java8=%3Cdefault%3E > > > > > > > > ср, 15 авг. 2018 г. в 20:49, Dmitriy Pavlov < > > > [hidden email]>: > > > > > > > > > > > > > > > > > > I've updated wiki page > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-C.CodeInspection > > > > > > > > > with > > > > > > > > > reference to settings.xml placement in the project. > > > > > > > > > > > > > > > > > > It is only advice, so I hope you don't mind having this > > > reference. > > > > > > > > > > > > > > > > > > ср, 15 авг. 2018 г. в 16:45, Dmitriy Pavlov < > > > [hidden email] > > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > > > > > > > Thank you for stepping in. I've committed the first > version > > > here > > > > > > > > > > 'idea/ignite_inspections.xml'. We can move it to project > > > default > > > > > > > > > > > > > > later > > > > > > > > when > > > > > > > > > > all inspection problems are fixed. > > > > > > > > > > Commit: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://git-wip-us.apache.org/repos/asf?p=ignite.git;a=commit;h=3e0f04edf7cc0aa1631fbd1b9af1e9b87b697eb1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > > > > > you can enable this profile using the following steps: > > > > > > > > > > Inspections > > > > > > > > > > (icon)->Configure inspections->(settings button)->Import > > > > > > > > > > > > > > > > Profile->select > > > > > > > > > > file and import. > > > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > > > вт, 14 авг. 2018 г. в 16:31, Maxim Muzafarov < > > > [hidden email] > > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > Dmitry and other Igniters, > > > > > > > > > > > > > > > > > > > > > > Previously you has suggested to commit `Code > Inspections` > > > into > > > > > > > > > > > > > > Ignite > > > > > > > > > > > codebase. > > > > > > > > > > > It makes sense for me. I think it's the easiest way to > > > share > > > > > > > > > > this > > > > > > > > profile > > > > > > > > > > > among community > > > > > > > > > > > members and this inspection can be set as for the > project > > > level. > > > > > > > > > > > So, I suggest: > > > > > > > > > > > > > > > > > > > > > > 1) According to Jetbrains documentation [1] the > > inspection > > > > > > > > > > profile > > > > > > > > can be > > > > > > > > > > > placed to > > > > > > > > > > > `<project>/.idea/inspectionProfiles` with name > > > > > > > > > > > > `Project_Default.xml` > > > > > > > > > > > (hope most of us using IDEA for development) > > > > > > > > > > > This allows enable this profile automatically on per > > > project > > > > > > > > > > level > > > > > > > and > > > > > > > > > > > will > > > > > > > > > > > simplify > > > > > > > > > > > development process according to rules accepted by our > > > > > > > > > > community. > > > > > > > > > > > > > > > > > > > > > > 2) I can file tickets and do some of them to fix > > inspection > > > > > > > > > > > > failures > > > > > > > > which > > > > > > > > > > > Alexey mentioned > > > > > > > > > > > earlier. Hope other members of community will help me > > with > > > it. > > > > > > > > > > > > > > > > > > > > > > 3) I think `Inspections (Core)` TeamCity can be > triggered > > > as > > > > > > > > > > > > nightly > > > > > > > > build > > > > > > > > > > > as it takes more > > > > > > > > > > > than 4 hours. Suppose, inspection build in each PR is > not > > > the > > > > > > > > > > best > > > > > > > > way in > > > > > > > > > > > our case. New run > > > > > > > > > > > here [2]. > > > > > > > > > > > > > > > > > > > > > > 4) We can tune our MTCGA.Bot to notify members with new > > > > > > > > > > inspection > > > > > > > > > > > failures > > > > > > > > > > > added by them. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, I've taken Alexey's inspection file as an > example, > > > I’ve > > > > > > > > > > > > > > checked > > > > > > > > what > > > > > > > > > > > we already discussed > > > > > > > > > > > previously (e.g. `Anonymous class can be converted to > > > lambda` > > > > > > > > > > > > should > > > > > > > > be > > > > > > > > > > > disabled by default) > > > > > > > > > > > and added these additional rules to it: > > > > > > > > > > > - General | Line is longer than allowed by code style > > > > > > > > > > > - Java | Code maturity | Call to 'printStackTrace()' > > > > > > > > > > > - Java | Code style issues | Unnecessary 'null' check > > > before > > > > > > > > > > > > > > > > 'equals()' > > > > > > > > > > > call > > > > > > > > > > > > > > > > > > > > > > If we decide to proceed I will attach this file to > JIRA. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > https://www.jetbrains.com/help/idea/code-inspection.html > > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3710%2Fhead > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 14 Aug 2018 at 16:19 Dmitriy Pavlov < > > > > > > > > > > > > [hidden email]> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Dmitriy Govoruknin, > > > > > > > > > > > > > > > > > > > > > > > > It seems there is a lack of volunteers to apply code > > > > > > > > > > inspections > > > > > > > to > > > > > > > > our > > > > > > > > > > > CI > > > > > > > > > > > > & patch submission process. Probably we could come > back > > > to > > > > > > > > > > your > > > > > > > > > > > > initial idea about setting up inspection locally. > > > > > > > > > > > > > > > > > > > > > > > > Could you commit or share your IDEA inspection > > settings? > > > I > > > > > > > > > > could > > > > > > > > apply > > > > > > > > > > > it > > > > > > > > > > > > at least on my machine and remove odd warning types > one > > > by > > > > > > > > > > one. > > > > > > > > What do > > > > > > > > > > > you > > > > > > > > > > > > think? > > > > > > > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > > > > > > > сб, 4 авг. 2018 г. в 1:22, Dmitriy Pavlov < > > > > > > > > > > > > [hidden email] > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > > > > Ideally, I should not asking for people to solve > > > something > > > > > > > > > > for > > > > > > > me. > > > > > > > > > > > > > > > > > > > > > > > > > > I just want this idea did not disappear from our > > radar. > > > > > > > > > > > > > > > > > > > > > > > > > > пт, 3 авг. 2018 г. в 23:47, Dmitriy Setrakyan < > > > > > > > > > > > > > > > > [hidden email] > > > > > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 3, 2018 at 7:49 AM, Dmitriy Pavlov < > > > > > > > > > > > > > > > > > > > > > > [hidden email]> > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I understand it is not so Apache-way from my > side > > > to ask > > > > > > > > > > > > > > > > > > > > > > volunteers to > > > > > > > > > > > > > > do > > > > > > > > > > > > > > > some things (instead of contributing it by > > myself). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Dmitriy, I am not sure why you feel this is not > the > > > Apache > > > > > > > > > > > > way. > > > > > > > > No > > > > > > > > > > > one > > > > > > > > > > > > can > > > > > > > > > > > > > > do everything themselves. You should absolutely > > keep > > > > > > > > > > > > > > recruiting > > > > > > > > more > > > > > > > > > > > > > > volunteers from the community. > > > > > > > > > > > > > > > > > > > > > > > > > > > > D. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > -- > > > > > > > > > > > Maxim Muzafarov > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -- > > > > > > Maxim Muzafarov > > > > > > > > > > > -- > > -- > > Maxim Muzafarov > > > > > -- > BR, Sergey Antonov > -- Maxim Muzafarov |
|
|
> On 26 Oct 2018, at 12:02, Maxim Muzafarov <[hidden email]> wrote: > > Petr, > >> Disclaimer: currently, on branches with no inspections file there is > default inspections file with all inspections turned off > > I'd suggest another approach: > - we should use the default inspection configuration file only for old > release branches like (2.5, 2.6 etc.); That’s exactly what was meant. > - the other branches should merge the latest changes from the master > branch and fix all local issues according to configured inspection rules; There is no easy way to distinguish obsolete branches from old release branch. Considering speed of changes applied to master branch, all branches either way will be updated to master soon. > > The other situations may lead us to new inspection issues come from merged > obsolete PRs. Also I really do not see any problem with that due to new functionality of such inspections and their influence on Test Run at all. As new way of testing code, this process will take it’s time to become common, and then there will be no obsolete branches. > > On Fri, 26 Oct 2018 at 11:53 Sergey Antonov <[hidden email]> > wrote: > >> Hello, Maxim! >> >> Good job! >> >> пт, 26 окт. 2018 г. в 0:47, Maxim Muzafarov <[hidden email]>: >> >>> Igniters, >>> >>> The new `Inspections: Core` suite [2] configured on TeamCity (pass >>> successfully with - Inspections total: 0, errors: 0 ). >>> The next rules are enabled for this suite: >>> - `Missorted modifiers`; >>> - `'size() == 0' replaceable with 'isEmpty()'`; >>> - `Add missing @Override annotation`; >>> - `Fix unused imports`; >>> >>> Let's incule it to the `Run::All` group on TC, so we will check these >> rules >>> automatically for each PR. >>> Any objections? >>> >>> >>> Talking about the details, >>> >>> - the issue [1] with adding an inspections configuration for TC have PA >>> status; >>> - the new configuration ignite_inspections_teamcity.xml added to PR; >>> - four rules which are already fixed in the master branch enabled in >>> config; >>> - the `Inspections:Core` suite configured to use the inspections >>> configuration from the local branch; >>> - the example `how to use inspections from the command line` added. >>> >>> >>> Petr, Nikolay, >>> >>> Thank you for your support! >>> >>> [1] https://issues.apache.org/jira/browse/IGNITE-9983 >>> [2] >>> >>> >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=pull%2F5059%2Fhead&tab=buildTypeStatusDiv >>> [3] >>> >>> >> https://issues.apache.org/jira/browse/IGNITE-9983?focusedCommentId=16662323&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16662323 >>> >>> On Tue, 23 Oct 2018 at 19:16 Nikolay Izhikov <[hidden email]> >> wrote: >>> >>>> Hello, Maxim. >>>> >>>> +1 from me. >>>> >>>> I vote to enable static inspections for the Ignite codebase. >>>> Thank you for that contributions! >>>> >>>> В Вт, 23/10/2018 в 19:09 +0300, Maxim Muzafarov пишет: >>>>> Igniters, >>>>> >>>>> I've fixed some issues according to the inspections.xml >> configuration: >>>>> - `Missorted modifiers`; >>>>> - `'size() == 0' replaceable with 'isEmpty()'`; >>>>> - `Add missing @Override annotation`; >>>>> These one have `In progress` state: >>>>> - `Fix unused imports`; >>>>> - `Remove unnecessary @SuppressWarnings annotation`; >>>>> >>>>> The list of issues related to the current Code Inspections changes >> can >>> be >>>>> found [1] >>>>> with using label `inspections`. So, to move forward and not lose >>> current >>>>> changes I >>>>> propose to: >>>>> - Create the new configuration idea\ignite_inspections_teamcity.xml >> (I >>>>> will file a new issue for it); >>>>> - Tune `Inspections: Core` Suite to use this configuration profile >> (It >>>>> will run with each PR); >>>>> - In the case with fixing a new inspection rule enable it this >>>> inspection >>>>> configuration. >>>>> >>>>> This will allow us to move forward in small steps and at some point >> of >>>> time >>>>> in future we will switch >>>>> this ignite_inspections_teamcity.xml with the >>>>> default ignite_inspections.xml. >>>>> >>>>> Thoughts? >>>>> Pert Ivanov, will you help to tune `Inspections: Core` suite? >>>>> >>>>> [1] >>>>> >>>> >>> >> https://issues.apache.org/jira/browse/IGNITE-9923?jql=project%20%3D%20Ignite%20AND%20labels%20%3D%20inspections >>>>> >>>>> On Sat, 25 Aug 2018 at 00:54 Dmitriy Pavlov <[hidden email]> >>>> wrote: >>>>> >>>>>> IntelliJ Idea shows missing @Override annotation on my >> installation. >>>> Not >>>>>> sure it comes from our inspection or not. >>>>>> >>>>>> Anyway, count on me. >>>>>> >>>>>> пт, 24 авг. 2018 г. в 9:25, Maxim Muzafarov <[hidden email]>: >>>>>> >>>>>>> Folks, >>>>>>> >>>>>>> I think we can make a small step further with Ignite Inspections. >>>>>>> >>>>>>> I've created these tickets [1], [2] for myself according to >>>> previously >>>>>>> added >>>>>>> `idea/ignite_inspections.xml` and I plan to complete them. >>>>>>> >>>>>>> Who will help me with review and merge? >>>>>>> >>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-9311 - Add >>> missing >>>>>>> @Override annotation >>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-9312 - Remove >>>>>> >>>>>> unnecessary >>>>>>> @SuppressWarnings annotation >>>>>>> >>>>>>> On Thu, 16 Aug 2018 at 19:53 Dmitriy Pavlov < >> [hidden email] >>>> >>>>>> >>>>>> wrote: >>>>>>> >>>>>>>> Hi Pavel, >>>>>>>> >>>>>>>> Thank you for noticing and bringing it here. I've fixed TC >>> failure. >>>>>>>> >>>>>>>> Sincerely, >>>>>>>> Dmitriy Pavlov >>>>>>>> >>>>>>>> чт, 16 авг. 2018 г. в 0:10, Pavel Pereslegin <[hidden email] >>> : >>>>>>>> >>>>>>>>> Hello Igniters. >>>>>>>>> >>>>>>>>> It seems that "idea/ignite_inspections.xml" should be >> excluded >>>> from >>>>>>>>> "check-licenses" maven profile, because "_Licenses Headers_" >>>>>>>>> configuration always fails now [1] on TeamCity. >>>>>>>>> >>>>>>>>> [1] >>>>>>>>> >>>>>> >>>>>> >>>> >>> >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_LicensesHeaders&tab=buildTypeHistoryList&branch_IgniteTests24Java8=%3Cdefault%3E >>>>>>>>> ср, 15 авг. 2018 г. в 20:49, Dmitriy Pavlov < >>>> [hidden email]>: >>>>>>>>>> >>>>>>>>>> I've updated wiki page >>>>>>>>>> >>>>>> >>>>>> >>>> >>> >> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-C.CodeInspection >>>>>>>>>> with >>>>>>>>>> reference to settings.xml placement in the project. >>>>>>>>>> >>>>>>>>>> It is only advice, so I hope you don't mind having this >>>> reference. >>>>>>>>>> >>>>>>>>>> ср, 15 авг. 2018 г. в 16:45, Dmitriy Pavlov < >>>> [hidden email] >>>>>>> >>>>>>> : >>>>>>>>>> >>>>>>>>>>> Hi Maxim, >>>>>>>>>>> >>>>>>>>>>> Thank you for stepping in. I've committed the first >> version >>>> here >>>>>>>>>>> 'idea/ignite_inspections.xml'. We can move it to project >>>> default >>>>>>>> >>>>>>>> later >>>>>>>>> when >>>>>>>>>>> all inspection problems are fixed. >>>>>>>>>>> Commit: >>>>>>>>>>> >>>>>>>>>>> >>>>>> >>>>>> >>>> >>> >> https://git-wip-us.apache.org/repos/asf?p=ignite.git;a=commit;h=3e0f04edf7cc0aa1631fbd1b9af1e9b87b697eb1 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Igniters, >>>>>>>>>>> >>>>>>>>>>> you can enable this profile using the following steps: >>>>>> >>>>>> Inspections >>>>>>>>>>> (icon)->Configure inspections->(settings button)->Import >>>>>>>>> >>>>>>>>> Profile->select >>>>>>>>>>> file and import. >>>>>>>>>>> >>>>>>>>>>> Sincerely, >>>>>>>>>>> Dmitriy Pavlov >>>>>>>>>>> >>>>>>>>>>> вт, 14 авг. 2018 г. в 16:31, Maxim Muzafarov < >>>> [hidden email] >>>>>>> >>>>>>> : >>>>>>>>>>> >>>>>>>>>>>> Dmitry and other Igniters, >>>>>>>>>>>> >>>>>>>>>>>> Previously you has suggested to commit `Code >> Inspections` >>>> into >>>>>>>> >>>>>>>> Ignite >>>>>>>>>>>> codebase. >>>>>>>>>>>> It makes sense for me. I think it's the easiest way to >>>> share >>>>>> >>>>>> this >>>>>>>>> profile >>>>>>>>>>>> among community >>>>>>>>>>>> members and this inspection can be set as for the >> project >>>> level. >>>>>>>>>>>> So, I suggest: >>>>>>>>>>>> >>>>>>>>>>>> 1) According to Jetbrains documentation [1] the >>> inspection >>>>>> >>>>>> profile >>>>>>>>> can be >>>>>>>>>>>> placed to >>>>>>>>>>>> `<project>/.idea/inspectionProfiles` with name >>>>>>> >>>>>>> `Project_Default.xml` >>>>>>>>>>>> (hope most of us using IDEA for development) >>>>>>>>>>>> This allows enable this profile automatically on per >>>> project >>>>>> >>>>>> level >>>>>>>> and >>>>>>>>>>>> will >>>>>>>>>>>> simplify >>>>>>>>>>>> development process according to rules accepted by our >>>>>> >>>>>> community. >>>>>>>>>>>> >>>>>>>>>>>> 2) I can file tickets and do some of them to fix >>> inspection >>>>>>> >>>>>>> failures >>>>>>>>> which >>>>>>>>>>>> Alexey mentioned >>>>>>>>>>>> earlier. Hope other members of community will help me >>> with >>>> it. >>>>>>>>>>>> >>>>>>>>>>>> 3) I think `Inspections (Core)` TeamCity can be >> triggered >>>> as >>>>>>> >>>>>>> nightly >>>>>>>>> build >>>>>>>>>>>> as it takes more >>>>>>>>>>>> than 4 hours. Suppose, inspection build in each PR is >> not >>>> the >>>>>> >>>>>> best >>>>>>>>> way in >>>>>>>>>>>> our case. New run >>>>>>>>>>>> here [2]. >>>>>>>>>>>> >>>>>>>>>>>> 4) We can tune our MTCGA.Bot to notify members with new >>>>>> >>>>>> inspection >>>>>>>>>>>> failures >>>>>>>>>>>> added by them. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Also, I've taken Alexey's inspection file as an >> example, >>>> I’ve >>>>>>>> >>>>>>>> checked >>>>>>>>> what >>>>>>>>>>>> we already discussed >>>>>>>>>>>> previously (e.g. `Anonymous class can be converted to >>>> lambda` >>>>>>> >>>>>>> should >>>>>>>>> be >>>>>>>>>>>> disabled by default) >>>>>>>>>>>> and added these additional rules to it: >>>>>>>>>>>> - General | Line is longer than allowed by code style >>>>>>>>>>>> - Java | Code maturity | Call to 'printStackTrace()' >>>>>>>>>>>> - Java | Code style issues | Unnecessary 'null' check >>>> before >>>>>>>>> >>>>>>>>> 'equals()' >>>>>>>>>>>> call >>>>>>>>>>>> >>>>>>>>>>>> If we decide to proceed I will attach this file to >> JIRA. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> [1] >>>> https://www.jetbrains.com/help/idea/code-inspection.html >>>>>>>>>>>> [2] >>>>>>>>>>>> >>>>>>>>>>>> >>>>>> >>>>>> >>>> >>> >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3710%2Fhead >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Tue, 14 Aug 2018 at 16:19 Dmitriy Pavlov < >>>>>>> >>>>>>> [hidden email]> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Dmitriy Govoruknin, >>>>>>>>>>>>> >>>>>>>>>>>>> It seems there is a lack of volunteers to apply code >>>>>> >>>>>> inspections >>>>>>>> to >>>>>>>>> our >>>>>>>>>>>> CI >>>>>>>>>>>>> & patch submission process. Probably we could come >> back >>>> to >>>>>> >>>>>> your >>>>>>>>>>>>> initial idea about setting up inspection locally. >>>>>>>>>>>>> >>>>>>>>>>>>> Could you commit or share your IDEA inspection >>> settings? >>>> I >>>>>> >>>>>> could >>>>>>>>> apply >>>>>>>>>>>> it >>>>>>>>>>>>> at least on my machine and remove odd warning types >> one >>>> by >>>>>> >>>>>> one. >>>>>>>>> What do >>>>>>>>>>>> you >>>>>>>>>>>>> think? >>>>>>>>>>>>> >>>>>>>>>>>>> Sincerely, >>>>>>>>>>>>> Dmitriy Pavlov >>>>>>>>>>>>> >>>>>>>>>>>>> сб, 4 авг. 2018 г. в 1:22, Dmitriy Pavlov < >>>>>>> >>>>>>> [hidden email] >>>>>>>>> : >>>>>>>>>>>>> >>>>>>>>>>>>>> Ideally, I should not asking for people to solve >>>> something >>>>>> >>>>>> for >>>>>>>> me. >>>>>>>>>>>>>> >>>>>>>>>>>>>> I just want this idea did not disappear from our >>> radar. >>>>>>>>>>>>>> >>>>>>>>>>>>>> пт, 3 авг. 2018 г. в 23:47, Dmitriy Setrakyan < >>>>>>>>> >>>>>>>>> [hidden email] >>>>>>>>>>>>> : >>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Fri, Aug 3, 2018 at 7:49 AM, Dmitriy Pavlov < >>>>>>>>>>>> >>>>>>>>>>>> [hidden email]> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I understand it is not so Apache-way from my >> side >>>> to ask >>>>>>>>>>>> >>>>>>>>>>>> volunteers to >>>>>>>>>>>>>>> do >>>>>>>>>>>>>>>> some things (instead of contributing it by >>> myself). >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Dmitriy, I am not sure why you feel this is not >> the >>>> Apache >>>>>>> >>>>>>> way. >>>>>>>>> No >>>>>>>>>>>> one >>>>>>>>>>>>> can >>>>>>>>>>>>>>> do everything themselves. You should absolutely >>> keep >>>>>>>> >>>>>>>> recruiting >>>>>>>>> more >>>>>>>>>>>>>>> volunteers from the community. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> D. >>>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> -- >>>>>>>>>>>> Maxim Muzafarov >>>>>>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> -- >>>>>>> Maxim Muzafarov >>>>>>> >>>> >>> -- >>> -- >>> Maxim Muzafarov >>> >> >> >> -- >> BR, Sergey Antonov >> > -- > -- > Maxim Muzafarov |
|
|
Agree with Petr.
Maxim, what are our next steps? Can we add check for - line length - indents (tabs vs spaces) This may require some efforts (will it and how much?), but can we add check for: - log messages structure - log.warn() vs U.warn() - abbreviations for local variables and fields. And last question > - the new configuration ignite_inspections_teamcity.xml added to PR; Can this be installed locally by every contributor to check the code? Can we add this to setup steps we have on wiki? --Yakov |
|
|
Also, let me note that currently only modules/core/** is under inspection.
And in spite of the fact that core is the “heaviest" module in project. it seems that adding new inspections should be 2-way: new inspections and new modules (builds). > On 26 Oct 2018, at 17:41, Yakov Zhdanov <[hidden email]> wrote: > > Agree with Petr. > > Maxim, what are our next steps? Can we add check for > - line length > - indents (tabs vs spaces) Should not we start adding new inspections after current errors all have been eliminated? > > This may require some efforts (will it and how much?), but can we add check > for: > - log messages structure > - log.warn() vs U.warn() > - abbreviations for local variables and fields. > > And last question > >> - the new configuration ignite_inspections_teamcity.xml added to PR; > > Can this be installed locally by every contributor to check the code? Can > we add this to setup steps we have on wiki? > > --Yakov |
|
|
Pert,
> As new way of testing code, this process will take it’s time to become common, and then there will be no obsolete branches. I tend to agree with you. Let's stabilize the solution first. Yakov, > Maxim, what are our next steps? Can we add check for > - line length > - indents (tabs vs spaces) I'd like to focus on the ignite-core module first. I will prepare the short list of first-priority inspections to enable and we will discuss it here. After `Inspection:Core` stabilization and the discussion of proposed rules, I will file new issues to do. Rules you mention is doable to enable. > This may require some efforts (will it and how much?), but can we add check > for: > - log messages structure > - log.warn() vs U.warn() > - abbreviations for local variables and fields. I will try to find a simple solution for Ignite-specific code-style requirements. As for the abbreviations community decide to make them not mandatory [1]. > Can this be installed locally by every contributor to check the code? > Can we add this to setup steps we have on wiki? They are already mentioned in the `Coding Guidelines` community wiki page [2]. This ignite_inspections_teamcity.xml configuration added only for the run with TC and contains enabled only some rules of original idea\ignite_inspections.xml configuration. [1] http://apache-ignite-developers.2346864.n4.nabble.com/Abbreviation-code-style-requirement-tp36605.html [2] https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-C.CodeInspection On Fri, 26 Oct 2018 at 17:51 Petr Ivanov <[hidden email]> wrote: > Also, let me note that currently only modules/core/** is under inspection. > And in spite of the fact that core is the “heaviest" module in project. it > seems that adding new inspections should be 2-way: new inspections and new > modules (builds). > > > > On 26 Oct 2018, at 17:41, Yakov Zhdanov <[hidden email]> wrote: > > > > Agree with Petr. > > > > Maxim, what are our next steps? Can we add check for > > - line length > > - indents (tabs vs spaces) > > Should not we start adding new inspections after current errors all have > been eliminated? > > > > > > This may require some efforts (will it and how much?), but can we add > check > > for: > > - log messages structure > > - log.warn() vs U.warn() > > - abbreviations for local variables and fields. > > > > And last question > > > >> - the new configuration ignite_inspections_teamcity.xml added to PR; > > > > Can this be installed locally by every contributor to check the code? Can > > we add this to setup steps we have on wiki? > > > > --Yakov > > -- Maxim Muzafarov |
|
|
Maxim,
Thanks for response, let's do it the way you suggested. Please consider adding more checks - line endings. I think we should only have \n - ensure blank line in the end of file All these are code reviews issues I pointed out many times when reviewing conributions. It would be cool if we have TC build failing if there is any. Thanks! --Yakov |
|
Hi Maxim,
thank you for your efforts to make this happen. Keep the pace! Could you please provide an example of how Inspections can fail, so I or another contributor could implement support of these failures validation in the Tc Bot. Sincerely, Dmitriy Pavlov пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email]>: > Maxim, > > Thanks for response, let's do it the way you suggested. > > Please consider adding more checks > - line endings. I think we should only have \n > - ensure blank line in the end of file > > All these are code reviews issues I pointed out many times when reviewing > conributions. It would be cool if we have TC build failing if there is any. > > Thanks! > > --Yakov > |
|
|
Igniters,
Since the inspection rules are included in RunAll a few members of the community mentioned a wide distributed execution time on TC agents: - 1h:27m:38s publicagent17_9094 - 38m:04s publicagent17_9094 - 33m:29s publicagent17_9094 - 17m:13s publicagent17_9094 It seems that we should configure the resources distribution across TC containers. Can anyone take a look at it? I've also prepared the short list of rules to work on: + Inconsistent line separators (6 matches) + Problematic whitespace (4 matches) + expression.equals("literal")' rather than '"literal".equals(expression) (53 matches) + Unnecessary 'null' check before 'instanceof' expression or call (42 matches) + Redundant 'if' statement (69 matches) + Redundant interface declaration (28 matches) + Double negation (0 matches) + Unnecessary code block (472 matches) + Line is longer than allowed by code style (2614 matches) (Is it possible to implement?) WDYT? On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <[hidden email]> wrote: > > Hi Maxim, > > thank you for your efforts to make this happen. Keep the pace! > > Could you please provide an example of how Inspections can fail, so I or > another contributor could implement support of these failures validation in > the Tc Bot. > > Sincerely, > Dmitriy Pavlov > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email]>: > > > Maxim, > > > > Thanks for response, let's do it the way you suggested. > > > > Please consider adding more checks > > - line endings. I think we should only have \n > > - ensure blank line in the end of file > > > > All these are code reviews issues I pointed out many times when reviewing > > conributions. It would be cool if we have TC build failing if there is any. > > > > Thanks! > > > > --Yakov > > |
|
|
Yakov, Dmitry,
Which example of unsuccessful suite execution do we need? Does the current fail [1] in the master branch enough to configure notifications by TC.Bot? > Please consider adding more checks > - line endings. I think we should only have \n > - ensure blank line at the end of file It seems to me that `line endings` is easy to add, but for the `blank line at the end` we need as special regexp. Can we focus on built-in IntelliJ inspections at first and fix others special further? [1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> wrote: > > Igniters, > > Since the inspection rules are included in RunAll a few members of the > community mentioned a wide distributed execution time on TC agents: > - 1h:27m:38s publicagent17_9094 > - 38m:04s publicagent17_9094 > - 33m:29s publicagent17_9094 > - 17m:13s publicagent17_9094 > It seems that we should configure the resources distribution across TC > containers. Can anyone take a look at it? > > > I've also prepared the short list of rules to work on: > + Inconsistent line separators (6 matches) > + Problematic whitespace (4 matches) > + expression.equals("literal")' rather than > '"literal".equals(expression) (53 matches) > + Unnecessary 'null' check before 'instanceof' expression or call (42 matches) > + Redundant 'if' statement (69 matches) > + Redundant interface declaration (28 matches) > + Double negation (0 matches) > + Unnecessary code block (472 matches) > + Line is longer than allowed by code style (2614 matches) (Is it > possible to implement?) > > WDYT? > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <[hidden email]> wrote: > > > > Hi Maxim, > > > > thank you for your efforts to make this happen. Keep the pace! > > > > Could you please provide an example of how Inspections can fail, so I or > > another contributor could implement support of these failures validation in > > the Tc Bot. > > > > Sincerely, > > Dmitriy Pavlov > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email]>: > > > > > Maxim, > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > Please consider adding more checks > > > - line endings. I think we should only have \n > > > - ensure blank line in the end of file > > > > > > All these are code reviews issues I pointed out many times when reviewing > > > conributions. It would be cool if we have TC build failing if there is any. > > > > > > Thanks! > > > > > > --Yakov > > > |
|
|
Guys, why we have 2 different inspection files in the repo?
idea\ignite_inspections.xml idea\ignite_inspections_teamcity.xml AFAIK TeamCity is able to use the same inspection file with IDE. I've imported 'idea\ignite_inspections.xml' in the IDE, but now see inspection warnings for my PR on TC because of different rules. On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <[hidden email]> wrote: > > Yakov, Dmitry, > > Which example of unsuccessful suite execution do we need? > Does the current fail [1] in the master branch enough to configure > notifications by TC.Bot? > > > Please consider adding more checks > > - line endings. I think we should only have \n > > - ensure blank line at the end of file > > It seems to me that `line endings` is easy to add, but for the `blank > line at the end` we need as special regexp. Can we focus on built-in > IntelliJ inspections at first and fix others special further? > > [1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> wrote: > > > > Igniters, > > > > Since the inspection rules are included in RunAll a few members of the > > community mentioned a wide distributed execution time on TC agents: > > - 1h:27m:38s publicagent17_9094 > > - 38m:04s publicagent17_9094 > > - 33m:29s publicagent17_9094 > > - 17m:13s publicagent17_9094 > > It seems that we should configure the resources distribution across TC > > containers. Can anyone take a look at it? > > > > > > I've also prepared the short list of rules to work on: > > + Inconsistent line separators (6 matches) > > + Problematic whitespace (4 matches) > > + expression.equals("literal")' rather than > > '"literal".equals(expression) (53 matches) > > + Unnecessary 'null' check before 'instanceof' expression or call (42 matches) > > + Redundant 'if' statement (69 matches) > > + Redundant interface declaration (28 matches) > > + Double negation (0 matches) > > + Unnecessary code block (472 matches) > > + Line is longer than allowed by code style (2614 matches) (Is it > > possible to implement?) > > > > WDYT? > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <[hidden email]> wrote: > > > > > > Hi Maxim, > > > > > > thank you for your efforts to make this happen. Keep the pace! > > > > > > Could you please provide an example of how Inspections can fail, so I or > > > another contributor could implement support of these failures validation in > > > the Tc Bot. > > > > > > Sincerely, > > > Dmitriy Pavlov > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email]>: > > > > > > > Maxim, > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > Please consider adding more checks > > > > - line endings. I think we should only have \n > > > > - ensure blank line in the end of file > > > > > > > > All these are code reviews issues I pointed out many times when reviewing > > > > conributions. It would be cool if we have TC build failing if there is any. > > > > > > > > Thanks! > > > > > > > > --Yakov > > > > -- Best Regards, Vyacheslav D. |
|
|
Hello, Vyacheslav.
Yes, we have. Maxim Muzafarov, can you fix it, please? вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur [hidden email]: > Guys, why we have 2 different inspection files in the repo? > idea\ignite_inspections.xml > idea\ignite_inspections_teamcity.xml > > AFAIK TeamCity is able to use the same inspection file with IDE. > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now see > inspection warnings for my PR on TC because of different rules. > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <[hidden email]> > wrote: > > > > Yakov, Dmitry, > > > > Which example of unsuccessful suite execution do we need? > > Does the current fail [1] in the master branch enough to configure > > notifications by TC.Bot? > > > > > Please consider adding more checks > > > - line endings. I think we should only have \n > > > - ensure blank line at the end of file > > > > It seems to me that `line endings` is easy to add, but for the `blank > > line at the end` we need as special regexp. Can we focus on built-in > > IntelliJ inspections at first and fix others special further? > > > > [1] > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> > wrote: > > > > > > Igniters, > > > > > > Since the inspection rules are included in RunAll a few members of the > > > community mentioned a wide distributed execution time on TC agents: > > > - 1h:27m:38s publicagent17_9094 > > > - 38m:04s publicagent17_9094 > > > - 33m:29s publicagent17_9094 > > > - 17m:13s publicagent17_9094 > > > It seems that we should configure the resources distribution across TC > > > containers. Can anyone take a look at it? > > > > > > > > > I've also prepared the short list of rules to work on: > > > + Inconsistent line separators (6 matches) > > > + Problematic whitespace (4 matches) > > > + expression.equals("literal")' rather than > > > '"literal".equals(expression) (53 matches) > > > + Unnecessary 'null' check before 'instanceof' expression or call (42 > matches) > > > + Redundant 'if' statement (69 matches) > > > + Redundant interface declaration (28 matches) > > > + Double negation (0 matches) > > > + Unnecessary code block (472 matches) > > > + Line is longer than allowed by code style (2614 matches) (Is it > > > possible to implement?) > > > > > > WDYT? > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <[hidden email]> > wrote: > > > > > > > > Hi Maxim, > > > > > > > > thank you for your efforts to make this happen. Keep the pace! > > > > > > > > Could you please provide an example of how Inspections can fail, so > I or > > > > another contributor could implement support of these failures > validation in > > > > the Tc Bot. > > > > > > > > Sincerely, > > > > Dmitriy Pavlov > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email]>: > > > > > > > > > Maxim, > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > Please consider adding more checks > > > > > - line endings. I think we should only have \n > > > > > - ensure blank line in the end of file > > > > > > > > > > All these are code reviews issues I pointed out many times when > reviewing > > > > > conributions. It would be cool if we have TC build failing if > there is any. > > > > > > > > > > Thanks! > > > > > > > > > > --Yakov > > > > > > > > > -- > Best Regards, Vyacheslav D. > |
|
|
Igniters,
I propose to make inspection configuration default on the project level. I've created a new issue [1] for it. It can be easily done and recommend by IntelliJ documentation [2]. Thoughts? Vyacheslav, Can you share an example of your warnings? Currently, we have different inspection configurations: - ignite_inspections.xml - to import inspections as default and use it daily. - ignite_inspections_teamcity.xml - config to run it on TC. Only fixed rules in the project code are enabled. Each of these rules are marked with ERROR level. [1] https://issues.apache.org/jira/browse/IGNITE-10422 [2] https://www.jetbrains.com/help/idea/code-inspection.html On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <[hidden email]> wrote: > > Hello, Vyacheslav. > > Yes, we have. > > Maxim Muzafarov, can you fix it, please? > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur [hidden email]: > > > Guys, why we have 2 different inspection files in the repo? > > idea\ignite_inspections.xml > > idea\ignite_inspections_teamcity.xml > > > > AFAIK TeamCity is able to use the same inspection file with IDE. > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now see > > inspection warnings for my PR on TC because of different rules. > > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <[hidden email]> > > wrote: > > > > > > Yakov, Dmitry, > > > > > > Which example of unsuccessful suite execution do we need? > > > Does the current fail [1] in the master branch enough to configure > > > notifications by TC.Bot? > > > > > > > Please consider adding more checks > > > > - line endings. I think we should only have \n > > > > - ensure blank line at the end of file > > > > > > It seems to me that `line endings` is easy to add, but for the `blank > > > line at the end` we need as special regexp. Can we focus on built-in > > > IntelliJ inspections at first and fix others special further? > > > > > > [1] > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> > > wrote: > > > > > > > > Igniters, > > > > > > > > Since the inspection rules are included in RunAll a few members of the > > > > community mentioned a wide distributed execution time on TC agents: > > > > - 1h:27m:38s publicagent17_9094 > > > > - 38m:04s publicagent17_9094 > > > > - 33m:29s publicagent17_9094 > > > > - 17m:13s publicagent17_9094 > > > > It seems that we should configure the resources distribution across TC > > > > containers. Can anyone take a look at it? > > > > > > > > > > > > I've also prepared the short list of rules to work on: > > > > + Inconsistent line separators (6 matches) > > > > + Problematic whitespace (4 matches) > > > > + expression.equals("literal")' rather than > > > > '"literal".equals(expression) (53 matches) > > > > + Unnecessary 'null' check before 'instanceof' expression or call (42 > > matches) > > > > + Redundant 'if' statement (69 matches) > > > > + Redundant interface declaration (28 matches) > > > > + Double negation (0 matches) > > > > + Unnecessary code block (472 matches) > > > > + Line is longer than allowed by code style (2614 matches) (Is it > > > > possible to implement?) > > > > > > > > WDYT? > > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <[hidden email]> > > wrote: > > > > > > > > > > Hi Maxim, > > > > > > > > > > thank you for your efforts to make this happen. Keep the pace! > > > > > > > > > > Could you please provide an example of how Inspections can fail, so > > I or > > > > > another contributor could implement support of these failures > > validation in > > > > > the Tc Bot. > > > > > > > > > > Sincerely, > > > > > Dmitriy Pavlov > > > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email]>: > > > > > > > > > > > Maxim, > > > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > > > Please consider adding more checks > > > > > > - line endings. I think we should only have \n > > > > > > - ensure blank line in the end of file > > > > > > > > > > > > All these are code reviews issues I pointed out many times when > > reviewing > > > > > > conributions. It would be cool if we have TC build failing if > > there is any. > > > > > > > > > > > > Thanks! > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > -- > > Best Regards, Vyacheslav D. > > |
|
|
I'm totally with you in this decision, let's move the file.
вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <[hidden email]>: > Igniters, > > I propose to make inspection configuration default on the project > level. I've created a new issue [1] for it. It can be easily done and > recommend by IntelliJ documentation [2]. > Thoughts? > > > Vyacheslav, > > Can you share an example of your warnings? > Currently, we have different inspection configurations: > - ignite_inspections.xml - to import inspections as default and use it > daily. > - ignite_inspections_teamcity.xml - config to run it on TC. Only fixed > rules in the project code are enabled. Each of these rules are marked > with ERROR level. > > [1] https://issues.apache.org/jira/browse/IGNITE-10422 > [2] https://www.jetbrains.com/help/idea/code-inspection.html > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <[hidden email]> wrote: > > > > Hello, Vyacheslav. > > > > Yes, we have. > > > > Maxim Muzafarov, can you fix it, please? > > > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur [hidden email]: > > > > > Guys, why we have 2 different inspection files in the repo? > > > idea\ignite_inspections.xml > > > idea\ignite_inspections_teamcity.xml > > > > > > AFAIK TeamCity is able to use the same inspection file with IDE. > > > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now see > > > inspection warnings for my PR on TC because of different rules. > > > > > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <[hidden email]> > > > wrote: > > > > > > > > Yakov, Dmitry, > > > > > > > > Which example of unsuccessful suite execution do we need? > > > > Does the current fail [1] in the master branch enough to configure > > > > notifications by TC.Bot? > > > > > > > > > Please consider adding more checks > > > > > - line endings. I think we should only have \n > > > > > - ensure blank line at the end of file > > > > > > > > It seems to me that `line endings` is easy to add, but for the `blank > > > > line at the end` we need as special regexp. Can we focus on built-in > > > > IntelliJ inspections at first and fix others special further? > > > > > > > > [1] > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> > > > wrote: > > > > > > > > > > Igniters, > > > > > > > > > > Since the inspection rules are included in RunAll a few members of > the > > > > > community mentioned a wide distributed execution time on TC agents: > > > > > - 1h:27m:38s publicagent17_9094 > > > > > - 38m:04s publicagent17_9094 > > > > > - 33m:29s publicagent17_9094 > > > > > - 17m:13s publicagent17_9094 > > > > > It seems that we should configure the resources distribution > across TC > > > > > containers. Can anyone take a look at it? > > > > > > > > > > > > > > > I've also prepared the short list of rules to work on: > > > > > + Inconsistent line separators (6 matches) > > > > > + Problematic whitespace (4 matches) > > > > > + expression.equals("literal")' rather than > > > > > '"literal".equals(expression) (53 matches) > > > > > + Unnecessary 'null' check before 'instanceof' expression or call > (42 > > > matches) > > > > > + Redundant 'if' statement (69 matches) > > > > > + Redundant interface declaration (28 matches) > > > > > + Double negation (0 matches) > > > > > + Unnecessary code block (472 matches) > > > > > + Line is longer than allowed by code style (2614 matches) (Is it > > > > > possible to implement?) > > > > > > > > > > WDYT? > > > > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov < > [hidden email]> > > > wrote: > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > thank you for your efforts to make this happen. Keep the pace! > > > > > > > > > > > > Could you please provide an example of how Inspections can fail, > so > > > I or > > > > > > another contributor could implement support of these failures > > > validation in > > > > > > the Tc Bot. > > > > > > > > > > > > Sincerely, > > > > > > Dmitriy Pavlov > > > > > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <[hidden email] > >: > > > > > > > > > > > > > Maxim, > > > > > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > - line endings. I think we should only have \n > > > > > > > - ensure blank line in the end of file > > > > > > > > > > > > > > All these are code reviews issues I pointed out many times when > > > reviewing > > > > > > > conributions. It would be cool if we have TC build failing if > > > there is any. > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > > > > > > -- > > > Best Regards, Vyacheslav D. > > > > |
|
|
Hi,
Have someone tried to investigate the issue related to Inspection TC task execution time variation (from 0.5 up to 1,5 hours)? Can we enable GC logs for this task or may be even get CPU, Disk, Network metrics? Can someone check if there are unnecessary Idea plugins starts that can be safely disabled? On Tue, Nov 27, 2018 at 5:52 PM Dmitriy Pavlov <[hidden email]> wrote: > I'm totally with you in this decision, let's move the file. > > вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <[hidden email]>: > > > Igniters, > > > > I propose to make inspection configuration default on the project > > level. I've created a new issue [1] for it. It can be easily done and > > recommend by IntelliJ documentation [2]. > > Thoughts? > > > > > > Vyacheslav, > > > > Can you share an example of your warnings? > > Currently, we have different inspection configurations: > > - ignite_inspections.xml - to import inspections as default and use it > > daily. > > - ignite_inspections_teamcity.xml - config to run it on TC. Only fixed > > rules in the project code are enabled. Each of these rules are marked > > with ERROR level. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10422 > > [2] https://www.jetbrains.com/help/idea/code-inspection.html > > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <[hidden email]> > wrote: > > > > > > Hello, Vyacheslav. > > > > > > Yes, we have. > > > > > > Maxim Muzafarov, can you fix it, please? > > > > > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur [hidden email]: > > > > > > > Guys, why we have 2 different inspection files in the repo? > > > > idea\ignite_inspections.xml > > > > idea\ignite_inspections_teamcity.xml > > > > > > > > AFAIK TeamCity is able to use the same inspection file with IDE. > > > > > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now see > > > > inspection warnings for my PR on TC because of different rules. > > > > > > > > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <[hidden email]> > > > > wrote: > > > > > > > > > > Yakov, Dmitry, > > > > > > > > > > Which example of unsuccessful suite execution do we need? > > > > > Does the current fail [1] in the master branch enough to configure > > > > > notifications by TC.Bot? > > > > > > > > > > > Please consider adding more checks > > > > > > - line endings. I think we should only have \n > > > > > > - ensure blank line at the end of file > > > > > > > > > > It seems to me that `line endings` is easy to add, but for the > `blank > > > > > line at the end` we need as special regexp. Can we focus on > built-in > > > > > IntelliJ inspections at first and fix others special further? > > > > > > > > > > [1] > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> > > > > wrote: > > > > > > > > > > > > Igniters, > > > > > > > > > > > > Since the inspection rules are included in RunAll a few members > of > > the > > > > > > community mentioned a wide distributed execution time on TC > agents: > > > > > > - 1h:27m:38s publicagent17_9094 > > > > > > - 38m:04s publicagent17_9094 > > > > > > - 33m:29s publicagent17_9094 > > > > > > - 17m:13s publicagent17_9094 > > > > > > It seems that we should configure the resources distribution > > across TC > > > > > > containers. Can anyone take a look at it? > > > > > > > > > > > > > > > > > > I've also prepared the short list of rules to work on: > > > > > > + Inconsistent line separators (6 matches) > > > > > > + Problematic whitespace (4 matches) > > > > > > + expression.equals("literal")' rather than > > > > > > '"literal".equals(expression) (53 matches) > > > > > > + Unnecessary 'null' check before 'instanceof' expression or call > > (42 > > > > matches) > > > > > > + Redundant 'if' statement (69 matches) > > > > > > + Redundant interface declaration (28 matches) > > > > > > + Double negation (0 matches) > > > > > > + Unnecessary code block (472 matches) > > > > > > + Line is longer than allowed by code style (2614 matches) (Is it > > > > > > possible to implement?) > > > > > > > > > > > > WDYT? > > > > > > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov < > > [hidden email]> > > > > wrote: > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > thank you for your efforts to make this happen. Keep the pace! > > > > > > > > > > > > > > Could you please provide an example of how Inspections can > fail, > > so > > > > I or > > > > > > > another contributor could implement support of these failures > > > > validation in > > > > > > > the Tc Bot. > > > > > > > > > > > > > > Sincerely, > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov < > [hidden email] > > >: > > > > > > > > > > > > > > > Maxim, > > > > > > > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > > - line endings. I think we should only have \n > > > > > > > > - ensure blank line in the end of file > > > > > > > > > > > > > > > > All these are code reviews issues I pointed out many times > when > > > > reviewing > > > > > > > > conributions. It would be cool if we have TC build failing if > > > > there is any. > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best Regards, Vyacheslav D. > > > > > > > -- Best regards, Andrey V. Mashenkov |
|
|
Hi, Maxim, Nikolay, I have the following questions regarding inspections:
Should 'gnite_inspections_teamcity.xml' been imported into IDEA, since 'ignite_inspections.xml' has been removed in actual master? Also, I've faced mismatching: if I use '@SuppressWarnings("ErrorNotRethrown")' in code, then this will be marked on TC as "Redundant suppression". If I removed this suppression in "main" code base (not in tests) then it's fine and IDE does not mark the code by inspection. But, if I use 'GridTestUtils#assertThrows' in 'tests' code base, then IDE requires to suppress the inspection, if I have done it then TC marks this as "Redundant suppression". What should I do in this case? On Mon, Dec 3, 2018 at 10:26 PM Andrey Mashenkov <[hidden email]> wrote: > > Hi, > > Have someone tried to investigate the issue related to Inspection TC task > execution time variation (from 0.5 up to 1,5 hours)? > Can we enable GC logs for this task or may be even get CPU, Disk, Network > metrics? > Can someone check if there are unnecessary Idea plugins starts that can be > safely disabled? > > > On Tue, Nov 27, 2018 at 5:52 PM Dmitriy Pavlov <[hidden email]> wrote: > > > I'm totally with you in this decision, let's move the file. > > > > вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <[hidden email]>: > > > > > Igniters, > > > > > > I propose to make inspection configuration default on the project > > > level. I've created a new issue [1] for it. It can be easily done and > > > recommend by IntelliJ documentation [2]. > > > Thoughts? > > > > > > > > > Vyacheslav, > > > > > > Can you share an example of your warnings? > > > Currently, we have different inspection configurations: > > > - ignite_inspections.xml - to import inspections as default and use it > > > daily. > > > - ignite_inspections_teamcity.xml - config to run it on TC. Only fixed > > > rules in the project code are enabled. Each of these rules are marked > > > with ERROR level. > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10422 > > > [2] https://www.jetbrains.com/help/idea/code-inspection.html > > > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <[hidden email]> > > wrote: > > > > > > > > Hello, Vyacheslav. > > > > > > > > Yes, we have. > > > > > > > > Maxim Muzafarov, can you fix it, please? > > > > > > > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur [hidden email]: > > > > > > > > > Guys, why we have 2 different inspection files in the repo? > > > > > idea\ignite_inspections.xml > > > > > idea\ignite_inspections_teamcity.xml > > > > > > > > > > AFAIK TeamCity is able to use the same inspection file with IDE. > > > > > > > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now see > > > > > inspection warnings for my PR on TC because of different rules. > > > > > > > > > > > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <[hidden email]> > > > > > wrote: > > > > > > > > > > > > Yakov, Dmitry, > > > > > > > > > > > > Which example of unsuccessful suite execution do we need? > > > > > > Does the current fail [1] in the master branch enough to configure > > > > > > notifications by TC.Bot? > > > > > > > > > > > > > Please consider adding more checks > > > > > > > - line endings. I think we should only have \n > > > > > > > - ensure blank line at the end of file > > > > > > > > > > > > It seems to me that `line endings` is easy to add, but for the > > `blank > > > > > > line at the end` we need as special regexp. Can we focus on > > built-in > > > > > > IntelliJ inspections at first and fix others special further? > > > > > > > > > > > > [1] > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <[hidden email]> > > > > > wrote: > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > Since the inspection rules are included in RunAll a few members > > of > > > the > > > > > > > community mentioned a wide distributed execution time on TC > > agents: > > > > > > > - 1h:27m:38s publicagent17_9094 > > > > > > > - 38m:04s publicagent17_9094 > > > > > > > - 33m:29s publicagent17_9094 > > > > > > > - 17m:13s publicagent17_9094 > > > > > > > It seems that we should configure the resources distribution > > > across TC > > > > > > > containers. Can anyone take a look at it? > > > > > > > > > > > > > > > > > > > > > I've also prepared the short list of rules to work on: > > > > > > > + Inconsistent line separators (6 matches) > > > > > > > + Problematic whitespace (4 matches) > > > > > > > + expression.equals("literal")' rather than > > > > > > > '"literal".equals(expression) (53 matches) > > > > > > > + Unnecessary 'null' check before 'instanceof' expression or call > > > (42 > > > > > matches) > > > > > > > + Redundant 'if' statement (69 matches) > > > > > > > + Redundant interface declaration (28 matches) > > > > > > > + Double negation (0 matches) > > > > > > > + Unnecessary code block (472 matches) > > > > > > > + Line is longer than allowed by code style (2614 matches) (Is it > > > > > > > possible to implement?) > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov < > > > [hidden email]> > > > > > wrote: > > > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > > > thank you for your efforts to make this happen. Keep the pace! > > > > > > > > > > > > > > > > Could you please provide an example of how Inspections can > > fail, > > > so > > > > > I or > > > > > > > > another contributor could implement support of these failures > > > > > validation in > > > > > > > > the Tc Bot. > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov < > > [hidden email] > > > >: > > > > > > > > > > > > > > > > > Maxim, > > > > > > > > > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > > > - line endings. I think we should only have \n > > > > > > > > > - ensure blank line in the end of file > > > > > > > > > > > > > > > > > > All these are code reviews issues I pointed out many times > > when > > > > > reviewing > > > > > > > > > conributions. It would be cool if we have TC build failing if > > > > > there is any. > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best Regards, Vyacheslav D. > > > > > > > > > > > > > -- > Best regards, > Andrey V. Mashenkov -- Best Regards, Vyacheslav D. |
|
|
Guys,
I've just creates a copy of Inspections TC build task with GC logs turned on to check if there is any issues and found Inspections task spent too much time in STW due to long Full GC pauses. I've tried to increase Xmx up to 4Gb and use G1GC got 2+ times better execution time down to ~15 min (~17 for 2G heap). Increasing heap size only is not very helpful as it just postpone Full GC issues, but changing GC to G1GC gives noticeable result. Let's apply this optimization. Thoughts? On Sun, Dec 9, 2018 at 12:43 PM Vyacheslav Daradur <[hidden email]> wrote: > Hi, Maxim, Nikolay, I have the following questions regarding inspections: > > Should 'gnite_inspections_teamcity.xml' been imported into IDEA, since > 'ignite_inspections.xml' has been removed in actual master? > > Also, I've faced mismatching: if I use > '@SuppressWarnings("ErrorNotRethrown")' in code, then this will be > marked on TC as "Redundant suppression". If I removed this suppression > in "main" code base (not in tests) then it's fine and IDE does not > mark the code by inspection. But, if I use > 'GridTestUtils#assertThrows' in 'tests' code base, then IDE requires > to suppress the inspection, if I have done it then TC marks this as > "Redundant suppression". > > What should I do in this case? > > On Mon, Dec 3, 2018 at 10:26 PM Andrey Mashenkov > <[hidden email]> wrote: > > > > Hi, > > > > Have someone tried to investigate the issue related to Inspection TC task > > execution time variation (from 0.5 up to 1,5 hours)? > > Can we enable GC logs for this task or may be even get CPU, Disk, Network > > metrics? > > Can someone check if there are unnecessary Idea plugins starts that can > be > > safely disabled? > > > > > > On Tue, Nov 27, 2018 at 5:52 PM Dmitriy Pavlov <[hidden email]> > wrote: > > > > > I'm totally with you in this decision, let's move the file. > > > > > > вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <[hidden email]>: > > > > > > > Igniters, > > > > > > > > I propose to make inspection configuration default on the project > > > > level. I've created a new issue [1] for it. It can be easily done and > > > > recommend by IntelliJ documentation [2]. > > > > Thoughts? > > > > > > > > > > > > Vyacheslav, > > > > > > > > Can you share an example of your warnings? > > > > Currently, we have different inspection configurations: > > > > - ignite_inspections.xml - to import inspections as default and use > it > > > > daily. > > > > - ignite_inspections_teamcity.xml - config to run it on TC. Only > fixed > > > > rules in the project code are enabled. Each of these rules are marked > > > > with ERROR level. > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10422 > > > > [2] https://www.jetbrains.com/help/idea/code-inspection.html > > > > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <[hidden email]> > > > wrote: > > > > > > > > > > Hello, Vyacheslav. > > > > > > > > > > Yes, we have. > > > > > > > > > > Maxim Muzafarov, can you fix it, please? > > > > > > > > > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur [hidden email] > : > > > > > > > > > > > Guys, why we have 2 different inspection files in the repo? > > > > > > idea\ignite_inspections.xml > > > > > > idea\ignite_inspections_teamcity.xml > > > > > > > > > > > > AFAIK TeamCity is able to use the same inspection file with IDE. > > > > > > > > > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now > see > > > > > > inspection warnings for my PR on TC because of different rules. > > > > > > > > > > > > > > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov < > [hidden email]> > > > > > > wrote: > > > > > > > > > > > > > > Yakov, Dmitry, > > > > > > > > > > > > > > Which example of unsuccessful suite execution do we need? > > > > > > > Does the current fail [1] in the master branch enough to > configure > > > > > > > notifications by TC.Bot? > > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > > - line endings. I think we should only have \n > > > > > > > > - ensure blank line at the end of file > > > > > > > > > > > > > > It seems to me that `line endings` is easy to add, but for the > > > `blank > > > > > > > line at the end` we need as special regexp. Can we focus on > > > built-in > > > > > > > IntelliJ inspections at first and fix others special further? > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > > > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov < > [hidden email]> > > > > > > wrote: > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > Since the inspection rules are included in RunAll a few > members > > > of > > > > the > > > > > > > > community mentioned a wide distributed execution time on TC > > > agents: > > > > > > > > - 1h:27m:38s publicagent17_9094 > > > > > > > > - 38m:04s publicagent17_9094 > > > > > > > > - 33m:29s publicagent17_9094 > > > > > > > > - 17m:13s publicagent17_9094 > > > > > > > > It seems that we should configure the resources distribution > > > > across TC > > > > > > > > containers. Can anyone take a look at it? > > > > > > > > > > > > > > > > > > > > > > > > I've also prepared the short list of rules to work on: > > > > > > > > + Inconsistent line separators (6 matches) > > > > > > > > + Problematic whitespace (4 matches) > > > > > > > > + expression.equals("literal")' rather than > > > > > > > > '"literal".equals(expression) (53 matches) > > > > > > > > + Unnecessary 'null' check before 'instanceof' expression or > call > > > > (42 > > > > > > matches) > > > > > > > > + Redundant 'if' statement (69 matches) > > > > > > > > + Redundant interface declaration (28 matches) > > > > > > > > + Double negation (0 matches) > > > > > > > > + Unnecessary code block (472 matches) > > > > > > > > + Line is longer than allowed by code style (2614 matches) > (Is it > > > > > > > > possible to implement?) > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov < > > > > [hidden email]> > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > > > > > thank you for your efforts to make this happen. Keep the > pace! > > > > > > > > > > > > > > > > > > Could you please provide an example of how Inspections can > > > fail, > > > > so > > > > > > I or > > > > > > > > > another contributor could implement support of these > failures > > > > > > validation in > > > > > > > > > the Tc Bot. > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov < > > > [hidden email] > > > > >: > > > > > > > > > > > > > > > > > > > Maxim, > > > > > > > > > > > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > > > > - line endings. I think we should only have \n > > > > > > > > > > - ensure blank line in the end of file > > > > > > > > > > > > > > > > > > > > All these are code reviews issues I pointed out many > times > > > when > > > > > > reviewing > > > > > > > > > > conributions. It would be cool if we have TC build > failing if > > > > > > there is any. > > > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best Regards, Vyacheslav D. > > > > > > > > > > > > > > > > > > > -- > > Best regards, > > Andrey V. Mashenkov > > > > -- > Best Regards, Vyacheslav D. > -- Best regards, Andrey V. Mashenkov |
|
|
Sure, let's apply. I hope all TC agents may handle 4G heap.
чт, 13 дек. 2018 г. в 12:54, Andrey Mashenkov <[hidden email]>: > Guys, > > I've just creates a copy of Inspections TC build task with GC logs turned > on to check if there is any issues > and found Inspections task spent too much time in STW due to long Full GC > pauses. > > I've tried to increase Xmx up to 4Gb and use G1GC got 2+ times better > execution time down to ~15 min (~17 for 2G heap). > Increasing heap size only is not very helpful as it just postpone Full GC > issues, but changing GC to G1GC gives noticeable result. > > Let's apply this optimization. > Thoughts? > > > On Sun, Dec 9, 2018 at 12:43 PM Vyacheslav Daradur <[hidden email]> > wrote: > > > Hi, Maxim, Nikolay, I have the following questions regarding inspections: > > > > Should 'gnite_inspections_teamcity.xml' been imported into IDEA, since > > 'ignite_inspections.xml' has been removed in actual master? > > > > Also, I've faced mismatching: if I use > > '@SuppressWarnings("ErrorNotRethrown")' in code, then this will be > > marked on TC as "Redundant suppression". If I removed this suppression > > in "main" code base (not in tests) then it's fine and IDE does not > > mark the code by inspection. But, if I use > > 'GridTestUtils#assertThrows' in 'tests' code base, then IDE requires > > to suppress the inspection, if I have done it then TC marks this as > > "Redundant suppression". > > > > What should I do in this case? > > > > On Mon, Dec 3, 2018 at 10:26 PM Andrey Mashenkov > > <[hidden email]> wrote: > > > > > > Hi, > > > > > > Have someone tried to investigate the issue related to Inspection TC > task > > > execution time variation (from 0.5 up to 1,5 hours)? > > > Can we enable GC logs for this task or may be even get CPU, Disk, > Network > > > metrics? > > > Can someone check if there are unnecessary Idea plugins starts that can > > be > > > safely disabled? > > > > > > > > > On Tue, Nov 27, 2018 at 5:52 PM Dmitriy Pavlov <[hidden email]> > > wrote: > > > > > > > I'm totally with you in this decision, let's move the file. > > > > > > > > вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <[hidden email]>: > > > > > > > > > Igniters, > > > > > > > > > > I propose to make inspection configuration default on the project > > > > > level. I've created a new issue [1] for it. It can be easily done > and > > > > > recommend by IntelliJ documentation [2]. > > > > > Thoughts? > > > > > > > > > > > > > > > Vyacheslav, > > > > > > > > > > Can you share an example of your warnings? > > > > > Currently, we have different inspection configurations: > > > > > - ignite_inspections.xml - to import inspections as default and use > > it > > > > > daily. > > > > > - ignite_inspections_teamcity.xml - config to run it on TC. Only > > fixed > > > > > rules in the project code are enabled. Each of these rules are > marked > > > > > with ERROR level. > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10422 > > > > > [2] https://www.jetbrains.com/help/idea/code-inspection.html > > > > > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <[hidden email] > > > > > > wrote: > > > > > > > > > > > > Hello, Vyacheslav. > > > > > > > > > > > > Yes, we have. > > > > > > > > > > > > Maxim Muzafarov, can you fix it, please? > > > > > > > > > > > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur > [hidden email] > > : > > > > > > > > > > > > > Guys, why we have 2 different inspection files in the repo? > > > > > > > idea\ignite_inspections.xml > > > > > > > idea\ignite_inspections_teamcity.xml > > > > > > > > > > > > > > AFAIK TeamCity is able to use the same inspection file with > IDE. > > > > > > > > > > > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now > > see > > > > > > > inspection warnings for my PR on TC because of different rules. > > > > > > > > > > > > > > > > > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov < > > [hidden email]> > > > > > > > wrote: > > > > > > > > > > > > > > > > Yakov, Dmitry, > > > > > > > > > > > > > > > > Which example of unsuccessful suite execution do we need? > > > > > > > > Does the current fail [1] in the master branch enough to > > configure > > > > > > > > notifications by TC.Bot? > > > > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > > > - line endings. I think we should only have \n > > > > > > > > > - ensure blank line at the end of file > > > > > > > > > > > > > > > > It seems to me that `line endings` is easy to add, but for > the > > > > `blank > > > > > > > > line at the end` we need as special regexp. Can we focus on > > > > built-in > > > > > > > > IntelliJ inspections at first and fix others special further? > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv > > > > > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov < > > [hidden email]> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > > > Since the inspection rules are included in RunAll a few > > members > > > > of > > > > > the > > > > > > > > > community mentioned a wide distributed execution time on TC > > > > agents: > > > > > > > > > - 1h:27m:38s publicagent17_9094 > > > > > > > > > - 38m:04s publicagent17_9094 > > > > > > > > > - 33m:29s publicagent17_9094 > > > > > > > > > - 17m:13s publicagent17_9094 > > > > > > > > > It seems that we should configure the resources > distribution > > > > > across TC > > > > > > > > > containers. Can anyone take a look at it? > > > > > > > > > > > > > > > > > > > > > > > > > > > I've also prepared the short list of rules to work on: > > > > > > > > > + Inconsistent line separators (6 matches) > > > > > > > > > + Problematic whitespace (4 matches) > > > > > > > > > + expression.equals("literal")' rather than > > > > > > > > > '"literal".equals(expression) (53 matches) > > > > > > > > > + Unnecessary 'null' check before 'instanceof' expression > or > > call > > > > > (42 > > > > > > > matches) > > > > > > > > > + Redundant 'if' statement (69 matches) > > > > > > > > > + Redundant interface declaration (28 matches) > > > > > > > > > + Double negation (0 matches) > > > > > > > > > + Unnecessary code block (472 matches) > > > > > > > > > + Line is longer than allowed by code style (2614 matches) > > (Is it > > > > > > > > > possible to implement?) > > > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov < > > > > > [hidden email]> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Maxim, > > > > > > > > > > > > > > > > > > > > thank you for your efforts to make this happen. Keep the > > pace! > > > > > > > > > > > > > > > > > > > > Could you please provide an example of how Inspections > can > > > > fail, > > > > > so > > > > > > > I or > > > > > > > > > > another contributor could implement support of these > > failures > > > > > > > validation in > > > > > > > > > > the Tc Bot. > > > > > > > > > > > > > > > > > > > > Sincerely, > > > > > > > > > > Dmitriy Pavlov > > > > > > > > > > > > > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov < > > > > [hidden email] > > > > > >: > > > > > > > > > > > > > > > > > > > > > Maxim, > > > > > > > > > > > > > > > > > > > > > > Thanks for response, let's do it the way you suggested. > > > > > > > > > > > > > > > > > > > > > > Please consider adding more checks > > > > > > > > > > > - line endings. I think we should only have \n > > > > > > > > > > > - ensure blank line in the end of file > > > > > > > > > > > > > > > > > > > > > > All these are code reviews issues I pointed out many > > times > > > > when > > > > > > > reviewing > > > > > > > > > > > conributions. It would be cool if we have TC build > > failing if > > > > > > > there is any. > > > > > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best Regards, Vyacheslav D. > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Best regards, > > > Andrey V. Mashenkov > > > > > > > > -- > > Best Regards, Vyacheslav D. > > > > > -- > Best regards, > Andrey V. Mashenkov > |
«
Return to Apache Ignite Developers - Legacy Mail Archive
|
1 view|%1 views
| Free forum by Nabble | Edit this page |