Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-8251

Create a Gradle task to use Xlint arguments when building

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: framework
    • Labels:
      None
    • Sprint:
      Community Day 3 - 2016
    1. OFBIZ-8251.patch
      2 kB
      Taher Alkhateeb
    2. OFBIZ-8251.patch
      0.8 kB
      Taher Alkhateeb

      Activity

      Hide
      taher Taher Alkhateeb added a comment - - edited

      Attached is a simple and clean fix. Essentially, it's better to just use Xlint which is equal to Xlint:all to show everything and you can dig through it for whatever you want.

      What this fix requires is only passing a project property called Xlint. So .. the syntax is ./gradlew -PXlint build

      For the record, I'm not very convinced of adding both Xlint and OWASP to the master build.gradle. I am busy at the moment but I do wish to remove both (even though I helped in implementing both) because I was just trying to help Jacques to implement it properly

      Show
      taher Taher Alkhateeb added a comment - - edited Attached is a simple and clean fix. Essentially, it's better to just use Xlint which is equal to Xlint:all to show everything and you can dig through it for whatever you want. What this fix requires is only passing a project property called Xlint. So .. the syntax is ./gradlew -PXlint build For the record, I'm not very convinced of adding both Xlint and OWASP to the master build.gradle. I am busy at the moment but I do wish to remove both (even though I helped in implementing both) because I was just trying to help Jacques to implement it properly
      Hide
      taher Taher Alkhateeb added a comment -

      Attaching a newer patch that also updated the README.md file for both xlint and owasp

      Show
      taher Taher Alkhateeb added a comment - Attaching a newer patch that also updated the README.md file for both xlint and owasp
      Hide
      jacques.le.roux Jacques Le Roux added a comment -

      Hi Taher,

      How do we know about the syntax apart checking in build.gradle? I thought about something more obvious like buildXlint which would be documented with gradlew tasks.

      For the record, I'm not very convinced of adding both Xlint and OWASP to the master build.gradle. I am busy at the moment but I do wish to remove both (even though I helped in implementing both) because I was just trying to help Jacques to implement it properly

      What do you then plan to do with Xlint and OWASP?

      Show
      jacques.le.roux Jacques Le Roux added a comment - Hi Taher, How do we know about the syntax apart checking in build.gradle? I thought about something more obvious like buildXlint which would be documented with gradlew tasks. For the record, I'm not very convinced of adding both Xlint and OWASP to the master build.gradle. I am busy at the moment but I do wish to remove both (even though I helped in implementing both) because I was just trying to help Jacques to implement it properly What do you then plan to do with Xlint and OWASP?
      Hide
      jacques.le.roux Jacques Le Roux added a comment -

      Taher, ah we crossed on wire about the documenation part, thanks!

      Show
      jacques.le.roux Jacques Le Roux added a comment - Taher, ah we crossed on wire about the documenation part, thanks!
      Hide
      taher Taher Alkhateeb added a comment -
      • In the newer patch, you will notice that I added the documentation for how to use Xlint in README.md
      • I don't plan to do anything, this is a minor issue at the moment, but I would like to start a discussion later on of whether to include what I think to be unnecessary features. This is a classic symptom of scope creep and would eventually lead to big ugly scripts exactly like the ones we had in the Ant days. Both OWASP and this feature were not discussed with the community to see its merit, and were just added hastily into the build script.
      Show
      taher Taher Alkhateeb added a comment - In the newer patch, you will notice that I added the documentation for how to use Xlint in README.md I don't plan to do anything, this is a minor issue at the moment, but I would like to start a discussion later on of whether to include what I think to be unnecessary features. This is a classic symptom of scope creep and would eventually lead to big ugly scripts exactly like the ones we had in the Ant days. Both OWASP and this feature were not discussed with the community to see its merit, and were just added hastily into the build script.
      Hide
      taher Taher Alkhateeb added a comment -

      Yup, got that

      Show
      taher Taher Alkhateeb added a comment - Yup, got that
      Hide
      jacques.le.roux Jacques Le Roux added a comment -

      Thanks Taher for your patch, committed in trunk at revision: 1761131

      Show
      jacques.le.roux Jacques Le Roux added a comment - Thanks Taher for your patch, committed in trunk at revision: 1761131

        People

        • Assignee:
          jacques.le.roux Jacques Le Roux
          Reporter:
          jacques.le.roux Jacques Le Roux
        • Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development

              Agile