Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-2020

Add Gradle RAT plugin to the top-level project

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0
    • Fix Version/s: 1.1.0
    • Component/s: build
    • Labels:
      None

      Description

      Apache Rat is used by Bigtop to find license violations. To avoid the mistake of failing to run rat from Maven, we should incorporate the upcoming Gradle plugin for Rat into the data generators build.

      1. BIGTOP-2020.2.patch
        3 kB
        RJ Nowling
      2. BIGTOP-2020.patch
        9 kB
        RJ Nowling

        Issue Links

          Activity

          Hide
          rnowling RJ Nowling added a comment -

          Committed!

          Show
          rnowling RJ Nowling added a comment - Committed!
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks for addressing the comments! +1

          Show
          cos Konstantin Boudnik added a comment - Thanks for addressing the comments! +1
          Hide
          rnowling RJ Nowling added a comment -

          Thanks for the review, Konstantin Boudnik!. The updated patch:

          • Does not include any changes to ASCII art
          • Uses only spaces for indentation (fixing the whitespace error)
          • Changes the excludes indentation level to be consistent with other lists that span multiple lines
          • Adds dl/** to the excludes list
          Show
          rnowling RJ Nowling added a comment - Thanks for the review, Konstantin Boudnik !. The updated patch: Does not include any changes to ASCII art Uses only spaces for indentation (fixing the whitespace error) Changes the excludes indentation level to be consistent with other lists that span multiple lines Adds dl/** to the excludes list
          Hide
          cos Konstantin Boudnik added a comment -

          It looks good. Two comments:

          • the patch touches our cute ascii-art. Is it relevant or necessary? Looks like whitespace changes or something. Could you please look at it?
          • white space error is in here (on the .idea line)
            +rat {
            +  excludes = [ ".git/**",
            +              ".idea/**",
            
          • while we are at it: shall we also exclude dl/ from the checks?
            Thanks for fitting this in!
          Show
          cos Konstantin Boudnik added a comment - It looks good. Two comments: the patch touches our cute ascii-art. Is it relevant or necessary? Looks like whitespace changes or something. Could you please look at it? white space error is in here (on the .idea line) +rat { + excludes = [ ".git/**" , + ".idea/**" , while we are at it: shall we also exclude dl/ from the checks? Thanks for fitting this in!
          Hide
          rnowling RJ Nowling added a comment -

          Adds the Apache RAT plugin to Gradle using that plugin currently available in the Gradle plugin repository.

          The plugin can be run with

          {./gradlew rat}

          . It found files added in BIGTOP-1746 without Apache headers, leading me to file a new JIRA BIGTOP-2037 and notify the authors. Therefore, I'm saying it works.

          Show
          rnowling RJ Nowling added a comment - Adds the Apache RAT plugin to Gradle using that plugin currently available in the Gradle plugin repository. The plugin can be run with {./gradlew rat} . It found files added in BIGTOP-1746 without Apache headers, leading me to file a new JIRA BIGTOP-2037 and notify the authors. Therefore, I'm saying it works.
          Hide
          cos Konstantin Boudnik added a comment -

          Well, I have the Groovy repo checked-out locally anyway

          Show
          cos Konstantin Boudnik added a comment - Well, I have the Groovy repo checked-out locally anyway
          Hide
          aw Allen Wittenauer added a comment -

          FYI, Samza has support already in their build system. We've also got support for that version in test-patch now.

          Show
          aw Allen Wittenauer added a comment - FYI, Samza has support already in their build system. We've also got support for that version in test-patch now.
          Hide
          cos Konstantin Boudnik added a comment -

          BTW, looks like there's an unofficial version of it used by Groovy as Paul King mentioned in here

          Show
          cos Konstantin Boudnik added a comment - BTW, looks like there's an unofficial version of it used by Groovy as Paul King mentioned in here
          Hide
          rnowling RJ Nowling added a comment -

          +1 Let's do it for both the whole project and the data generators.

          Show
          rnowling RJ Nowling added a comment - +1 Let's do it for both the whole project and the data generators.
          Hide
          cos Konstantin Boudnik added a comment -

          let's do this for the whole project, if you don't mind!?

          Show
          cos Konstantin Boudnik added a comment - let's do this for the whole project, if you don't mind!?

            People

            • Assignee:
              rnowling RJ Nowling
              Reporter:
              rnowling RJ Nowling
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development