logging mechanism should be improved and unified

[yegor256]

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

a) to get resource validators from ValidatorsProvider (method to get such validators will be added in #44)
b) if a) returns any resource validators then get list of files to be checked (should be all files of the target project under src folder).
c) iterate over that list of files then iterate over each resource validator returned from a) and send current file to the current resource validator for validation (i.e.

call ResourceValidator.validate method).
d) If ResourceValidator.validate returns ResourceValidationResult that is failed then log returned error with
com.ymock.util.Logger.error()
specifying validation resource (path should be relative to
this.session.getExecutionRootDirectory()), line number if present in validation result, error message if present in validation result.
e) after validation of all files is over if any validation failed throw MojoFailureException

---

- ~~`61-654de8aa`/#741~~ (by Krzysztof Krason) - `61-61fa67ee`/#742 (by ) - `61-1d39a5c7`/#746 (by Krzysztof Krason) - `61-832eca62`/#740 (by ) - `61-1e86e587`/#745 (by )

[yegor256]

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

Another redesign task to solve #42

[yegor256]

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

Let's wait for #44

[yegor256]

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

Thanks for reporting it, 1/2hrs are yours.

[yegor256]

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

I'm suggesting to improve it this way. First, Validator#validate() will have two params:

interface Validator {
  void validate(final Environment env, final Violations violations)
    throws ValidationException;
}

Then, we add this interface:

interface com.qulice.spi.Violations extends Collection<Violation> {
}

and a class

class com.qulice.spi.Violation {
  public Violation file(File file);
  public Violation line(Integer line);
  public Violation text(String text);
  public Violation check(String name);
}

Then, every validator will do literally this:

void validate(final Environment env, final Violations violations) {
  violations.add(
    new Violation()
      .file("/.../src/main/java/Test.java")
      .lines("34-56")
      .text("Too many methods in this class")
      .check("TooManyMethods")
  );
}

Looks good?

[yegor256]

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

Validators won't log any events directly any more. They will just add violations to the collection. All reporting will be done in one place, in CheckMojo.

[yegor256]

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

Moreover, we can get rid of ValidationException, and declare validate() like this:

interface Validator {
  Collection<Violation> validate(final Environment env);
}

Looks even better for me. What do you think?

[yegor256]

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

Dmitry, can you implement this refactoring? The budget is 1 hour.

[yegor256]

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

In [402/qulice]:

#CommitTicketReference repository="qulice" revision="402"
refs #61 - new branch

[yegor256]

migrated from Trac, where originally posted by dmitry.bashkin on 14-Dec-2011 2:36pm

What to do with files that are not compiled, for example CheckstyleValidator:
I mean what to with this:
throw new ValidationException(
"%d Checkstyle violations (see log above)",
events.size()

[yegor256]

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

Milestone PROTOTYPE deleted

[yegor256]

migrated from Trac, where originally posted by yegor256 on 1-Apr-2012 6:11pm

Dmitry is no longer in the project, I'll implement this myself

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 4:22am

In [503/qulice]:

#CommitTicketReference repository="qulice" revision="503"
refs #61 - new branch

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 4:23am

In [504/qulice]:

#CommitTicketReference repository="qulice" revision="504"
refs #61 - old branch terminated

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 4:23am

In [505/qulice]:

#CommitTicketReference repository="qulice" revision="505"
refs #61 - new branch

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 4:33am

In [506/qulice]:

#CommitTicketReference repository="qulice" revision="506"
refs #61 - SPI refactored

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 4:37am

In [507/qulice]:

#CommitTicketReference repository="qulice" revision="507"
refs #61 - XML module refactored

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 5:01am

In [508/qulice]:

#CommitTicketReference repository="qulice" revision="508"
refs #61 - temp commit

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 5:02am

In [509/qulice]:

#CommitTicketReference repository="qulice" revision="509"
refs #61 - renamed

[yegor256]

migrated from Trac, where originally posted by yegor256 on 2-Apr-2012 6:15am

In [510/qulice]:

#CommitTicketReference repository="qulice" revision="510"
refs #61 - temp commit

[yegor256]

migrated from Trac, where originally posted by yegor256 on 24-May-2012 1:14pm

let's do it in the next version

[yegor256]

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

we moved to Github

[krzyk]

@yegor256 is this still valid?

[yegor256]

@krzyk up to you. I think, the idea is interesting. you decide.

[krzyk]

@davvd it is a valid bug

[davvd]

@davvd it is a valid bug

@krzyk added "bug" tag to this issue

[davvd]

@yegor256 thank you for reporting this, I added 15 mins to your acc, transaction 000-1734463e

[davvd]

@davvd this is postponed

@krzyk sure, thanks, I added "postponed" label to it

[davvd]

@davvd this is postponed

@krzyk I will assign somebody else to this issue

[krzyk]

@davvd this is not postponed

[krzyk]

@davvd assign @krzyk

[davvd]

@davvd this is not postponed

@krzyk got it, "postponed" tag removed from here

[davvd]

@davvd assign @krzyk

@krzyk OK @krzyk please go ahead, this task is yours

[krzyk]

@yegor256 initial batch has been merged to master (checkstyle uses new logging, others are in todos), please close this issue if it is OK

[yegor256]

@krzyk thanks!

[davvd]

@yegor256 5 puzzles were created here: 61-61fa67ee, 61-1d39a5c7, 61-832eca62/#740, 61-1e86e587, 61-654de8aa

[davvd]

@krzyk I just added 45 mins to your account, many thanks for your contribution (79287961).. 45 hours and 39 mins spent here; there is a bonus for fast delivery (m=2739); +45 added to your rating, current score is: +5930

[0pdd]

@yegor256 the puzzle #1127 is still not solved.