Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      1

      Activity

      Hide
      Dennis Lundberg added a comment -

      Fixed a couple of minor typos and changed "checkstyle" to "Checkstyle" in a number of places.

      Show
      Dennis Lundberg added a comment - Fixed a couple of minor typos and changed "checkstyle" to "Checkstyle" in a number of places.
      Hide
      Maria Odea Ching added a comment -

      Review comments by Vincent Siveton (from Maven Dev List):

      ------
      here my comments:

      usage.html
      Should be better to create two subsections:

      • Generate the report as part of Project Reports
      • Generate the report as standalone

      Maybe add a report screenshot

      FAQ
      "checkstyle.properties" or "checkstyle properties" (with space)
      If the first, the following seems wrong according the doc
      <configLocation>checkstyle.properties</configLocation>
      http://people.apache.org/~oching/maven-checkstyle-plugin/checkstyle-mojo.html#configLocation

      custom-property-expansion.html
      propertiesLocation has no sample. Is is the same explained in FAQ?

      multi-module-config.html
      Review it as said
      Replace unix commands by a directory layout:
      whizbang

      – pom.xml
      `-- src
      – main
      ...

      Cheers,

      Vincent

      Show
      Maria Odea Ching added a comment - Review comments by Vincent Siveton (from Maven Dev List): ------ here my comments: usage.html Should be better to create two subsections: Generate the report as part of Project Reports Generate the report as standalone Maybe add a report screenshot FAQ "checkstyle.properties" or "checkstyle properties" (with space) If the first, the following seems wrong according the doc <configLocation>checkstyle.properties</configLocation> http://people.apache.org/~oching/maven-checkstyle-plugin/checkstyle-mojo.html#configLocation custom-property-expansion.html propertiesLocation has no sample. Is is the same explained in FAQ? multi-module-config.html Review it as said Replace unix commands by a directory layout: whizbang – pom.xml `-- src – main ... Cheers, Vincent
      Hide
      Maria Odea Ching added a comment -

      Review comments by Stephen Duncan (from Maven Dev List):


      On the "Multimodule Configuration" documentation:

      As I just mentioned on a question on the user's list, I don't think
      it's correct to specify the "build-tools" dependency as a dependency
      of the plugin. While this will work if you manually install the
      build-tools jar, it will not download it from an internal repository.
      It should instead be specified as build extension like in the "Using
      Custom Developed Chechstyle Check Modules" example. (Also not the
      spelling mistake in that title: ChecHstyle).

      Because this is somewhat confusing, I think it should mentioned either
      in the "Using a Custom Checkstyle Checker Configuration" as a way of
      using a classpath reference, or it should be it's own guide on using a
      shared jar for configuration.

      • Stephen
      Show
      Maria Odea Ching added a comment - Review comments by Stephen Duncan (from Maven Dev List): On the "Multimodule Configuration" documentation: As I just mentioned on a question on the user's list, I don't think it's correct to specify the "build-tools" dependency as a dependency of the plugin. While this will work if you manually install the build-tools jar, it will not download it from an internal repository. It should instead be specified as build extension like in the "Using Custom Developed Chechstyle Check Modules" example. (Also not the spelling mistake in that title: ChecHstyle). Because this is somewhat confusing, I think it should mentioned either in the "Using a Custom Checkstyle Checker Configuration" as a way of using a classpath reference, or it should be it's own guide on using a shared jar for configuration. Stephen
      Hide
      Maria Odea Ching added a comment -

      Review comments by Dennis Lundberg (from Maven Dev List):


      +1 to put the it in a guide of its own. I believe that this is a very common thing that companies and large organizations want to do.

      I've attached a path to MCHECKSTYLE-49 with some minor fixes.

      The goal descriptions are not clear to me as they are now. What is the difference between the goals? It sound like they do the same thing. Also their descriptions are not the same on the index page as on the plugin-info page.

      Show
      Maria Odea Ching added a comment - Review comments by Dennis Lundberg (from Maven Dev List): +1 to put the it in a guide of its own. I believe that this is a very common thing that companies and large organizations want to do. I've attached a path to MCHECKSTYLE-49 with some minor fixes. The goal descriptions are not clear to me as they are now. What is the difference between the goals? It sound like they do the same thing. Also their descriptions are not the same on the index page as on the plugin-info page.
      Hide
      Maria Odea Ching added a comment -

      I've already revised and applied the review comments in the plugin docs.

      Btw, for the shared jar configuration issue in "Multimodule Configuration", I've just revised the page and mentioned the shared jar configuration issue (declaring it as an extension instead of as a plugin dependency) instead of having a separate guide for that. I did this because I think it would just have almost the same example used in "Multimodule Configuration".

      The staging site (http://people.apache.org/~oching/maven-checkstyle-plugin) has already been updated. Thanks

      Show
      Maria Odea Ching added a comment - I've already revised and applied the review comments in the plugin docs. Btw, for the shared jar configuration issue in "Multimodule Configuration", I've just revised the page and mentioned the shared jar configuration issue (declaring it as an extension instead of as a plugin dependency) instead of having a separate guide for that. I did this because I think it would just have almost the same example used in "Multimodule Configuration". The staging site ( http://people.apache.org/~oching/maven-checkstyle-plugin ) has already been updated. Thanks

        People

        • Assignee:
          Maria Odea Ching
          Reporter:
          Maria Odea Ching
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Due:
            Created:
            Updated:
            Resolved:

            Time Tracking

            Estimated:
            Original Estimate - 22 hours Original Estimate - 22 hours
            22h
            Remaining:
            Remaining Estimate - 0 minutes
            0m
            Logged:
            Time Spent - 1 day, 7 hours
            1d 7h