optimize files discovery mechanism for all validators about qulice HOT 33 CLOSED

migrated from Trac, where originally posted by yegor256 on 7-Nov-2011 3:54pm

Could you please be more specific in this ticket. What exactly needs to be improved and where? I will set the budget when the scope is clear.

from qulice.

migrated from Trac, where originally posted by dpetruha on 7-Nov-2011 4:35pm

Yegor! Currently I need to know what validators we are supporting in code right now. I can see from here
com.qulice.maven.ValidatorsProvider.all()
that only these are active EnforcerValidator, CheckstyleValidator,PMDValidator,CodeNarcValidator. Some validators are commented there. So if we are about to change the design, should we do that also for not used validators?

from qulice.

migrated from Trac, where originally posted by yegor256 on 7-Nov-2011 4:38pm

Yes, all supported validators are listed in ValidatorsProvider.all(). I don't know what do you mean by "we're about to change the design".

from qulice.

migrated from Trac, where originally posted by yegor256 on 7-Nov-2011 4:40pm

In this ticket, if you want a change, you should explain what exactly is a problem in current design/implementation. And how this problem affects the product. I will review the explanation and will make a decision whether the change is required, who will do it, when, and what will be the budget.

But first, I need a detailed explanation of a problem, please.

from qulice.

migrated from Trac, where originally posted by dpetruha on 7-Nov-2011 5:01pm

I've described the problem here http://trac.fazend.com/qulice/ticket/42#comment:5 or you need some more detailed description?

from qulice.

migrated from Trac, where originally posted by yegor256 on 7-Nov-2011 5:05pm

You mentioned at least three problems in that comment. We are working with one problem per ticket always. And every ticket should be isolated from others. Thus, you need to re-explain in this one what exactly is wrong, please.

from qulice.

migrated from Trac, where originally posted by yegor256 on 20-Nov-2011 6:48pm

Is this ticket still active?

from qulice.

migrated from Trac, where originally posted by dpetruha on 24-Nov-2011 9:51am

This interface is supposed to be implemented by any validator supposed to validate file system resources.

a)
Should have one method:

ResourceValidationResult validate(File file, final Environment env).

Return value can be null in case validator is not interested in validation of the file passed.

b)ResourceValidationResult

should have methods to get validation status (Ok, Failed), line number in resource if present, error message.

c) Add method to get list of resource validators to com.qulice.maven.ValidatorsProvider in analogy with com.qulice.maven.ValidatorsProvider.all().

from qulice.

migrated from Trac, where originally posted by dpetruha on 24-Nov-2011 9:53am

Hi Yegor! This task is one of the redesign tasks I see to solve problem reported in #42.

from qulice.

migrated from Trac, where originally posted by yegor256 on 24-Nov-2011 11:23am

I got the idea, but how will we know which files a particular validator is interested in? For example, CodeNarcValidator is interested in *.groovy files. This knowledge is encapsulated/hidden inside the class. In your scenario we will have to move this information outside the class. Where will it be managed/located?

from qulice.

migrated from Trac, where originally posted by yegor256 on 24-Nov-2011 11:24am

Besides that, every validator knows where files are located. For example, PMD validator works with files in src/main/java directory.

from qulice.

migrated from Trac, where originally posted by dpetruha on 24-Nov-2011 11:31am

there is a note on that above in the comment (http://trac.fazend.com/qulice/ticket/44#comment:9)

Validator is supposed to null in case validator is not interested in validation of the file passed.

So CheckMojo.execute() passes each file in target project to each validator and in case validator does not care about that file returns null at once.

from qulice.

migrated from Trac, where originally posted by yegor256 on 27-Nov-2011 10:07am

OK, makes sense. Thanks for reporting this problem, 1/2hrs are yours. I will fix it today, but in a bit different way.

I will introduce a method in Environment interface: files(String match). This method will return a collection of files found according to the match pattern provided. For example:

env.files("src/main/java/*.java");
env.files("**/*.groovy");
env.files("src/main/java/*.java;src/test/java/*.java");

I think this design will be simpler and more effective. Agree?

from qulice.

migrated from Trac, where originally posted by yegor256 on 13-Dec-2011 1:03pm

Dmitry, can you implement this refactoring and introduce env.files() method? The budget is * 1 hour*.

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 8:10am

In [397/qulice]:

#CommitTicketReference repository="qulice" revision="397"
refs #44 - new branch

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 9:49am

In [400/qulice]:

#CommitTicketReference repository="qulice" revision="400"
refs #44 - done

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 10:00am

In [401/qulice]:

#CommitTicketReference repository="qulice" revision="401"
refs #44 - done

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 10:03am

Do you mean implement this method or only add method to interface?

from qulice.

migrated from Trac, where originally posted by yegor256 on 14-Dec-2011 10:22am

Can you just add it to the interface and make the build clean? :)

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 10:43am

In [403/qulice]:

#CommitTicketReference repository="qulice" revision="403"
refs #44 - done

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 10:44am

In [404/qulice]:

#CommitTicketReference repository="qulice" revision="404"
refs #44 - sync with trunk

from qulice.

migrated from Trac, where originally posted by yegor256 on 14-Dec-2011 10:53am

Good for a start, but what about @todo puzzles? If we merge this implementation to trunk and close this ticket - how we will know what to do next in order to finish the task. Ultimately we want all modules (checkstyle, pmd, findbugs, etc.) to use this method instead of direct file finding they use now (through FileUtils).

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 18-Dec-2011 3:21pm

In [405/qulice]:

#CommitTicketReference repository="qulice" revision="405"
refs #44 - done

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 18-Dec-2011 3:21pm

In [406/qulice]:

#CommitTicketReference repository="qulice" revision="406"
refs #44 - sync with trunk

from qulice.

migrated from Trac, where originally posted by yegor256 on 20-Dec-2011 8:14am

Let's try to merge...

from qulice.

migrated from Trac, where originally posted by yegor256 on 20-Dec-2011 8:30am

failed:

[INFO] --- qulice-maven-plugin:0.1.5:check (default) @ qulice-checkstyle ---
[INFO] Calling org.apache.maven.plugins:maven-enforcer-plugin:1.0-beta-1:enforce...
[INFO] LICENSE found: jar:file:/tmp/ccFacade-dir-1QbwOR/repo-trunk/qulice/qulice-tk/target/qulice-tk-1.0-SNAPSHOT.jar!/LICENSE.txt
[ERROR] /src/main/java/com/qulice/checkstyle/CheckstyleValidator.java[208]: One space indentation expected in @todo puzzle (PuzzleFormatCheck)
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] qulice-tk ......................................... SUCCESS [5.516s]
[INFO] qulice ............................................ SUCCESS [17.918s]
[INFO] qulice-spi ........................................ SUCCESS [18.243s]
[INFO] qulice-checkstyle ................................. FAILURE [23.958s]
[INFO] qulice-codenarc ................................... SKIPPED
[INFO] qulice-pmd ........................................ SKIPPED
[INFO] qulice-findbugs ................................... SKIPPED
[INFO] qulice-xml ........................................ SKIPPED
[INFO] qulice-maven-plugin ............................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

from qulice.

migrated from Trac, where originally posted by dmitry.bashkin on 20-Dec-2011 9:08am

In [407/qulice]:

#CommitTicketReference repository="qulice" revision="407"
refs #44 - done

from qulice.

migrated from Trac, where originally posted by yegor256 on 20-Dec-2011 12:11pm

let's try again

from qulice.

migrated from Trac, where originally posted by guard on 20-Dec-2011 12:23pm

In [408/qulice]:

#CommitTicketReference repository="qulice" revision="408"
refs #44 - /branches/ticket44 merged into /trunk, 1 hours spent

from qulice.

migrated from Trac, where originally posted by guard on 20-Dec-2011 12:23pm

In [409/qulice]:

#CommitTicketReference repository="qulice" revision="409"
refs #44 - /branches/ticket44 successfully closed

from qulice.

migrated from Trac, where originally posted by yegor256 on 1-Jan-2011 6:21am

thanks!

from qulice.

migrated from Trac, where originally posted by yegor256 on 22-Jan-2012 11:39pm

Milestone PROTOTYPE deleted

from qulice.

migrated from Trac, where originally posted by yegor on 21-Apr-2013 12:03pm

we moved to Github

from qulice.