Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added configuration options #30

Merged
merged 22 commits into from
Jan 22, 2018
Merged

Added configuration options #30

merged 22 commits into from
Jan 22, 2018

Conversation

jack-bradshaw
Copy link
Contributor

Added configuration options to allow scripts to configure the following properties in Android projects:

  • Enable/disable HTML report generation
  • Enable/disable HTML report copying to assets
  • Enable/disable JSON report generation
  • Enable/disable JSON report copying to assets

Added tests to check that options are used by task.

Matthew Tamlin added 7 commits January 21, 2018 13:32
* feature/enable-configuration:
  test(core): added tests for android tasks to check that options are respected
  feat(core): configured java task using constants for backwards compatibility
  feat(core): configured task for Android projects using options
  feat(core): added conditional logic to license generation/export based on inputs
  feat(core): added configuration options to license report task
  feat(configuration): added configuration class
@@ -30,6 +30,8 @@ final class LicensePlugin implements Plugin<Project> {
// Get correct plugin - Check for android library, default to application variant for application/test plugin
final variants = getAndroidVariants(project)

final configurationExtension = project.extensions.create("options", AndroidLicenseReportOptions)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably change options to something more clear such as licenseReport or just license.

compile DESIGN
}

project.extensions.configure(AndroidLicenseReportOptions, {options -> options.generateHtmlReport = false})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the DSL directly?

project.extensions.options {
  generateHtmlReport = false
  generateJsonReport = false
  copyHtmlReportToAssets = copyEnabled
  copyJsonReportToAssets = copyEnabled
}

@jaredsburrows
Copy link
Owner

jaredsburrows commented Jan 21, 2018

This is look great just a few comments and we need to update README.md for documentation.

Matthew Tamlin added 15 commits January 21, 2018 22:20
* feature/enable-configuration:
  doc(readme): added instructions for configuring the report tasks
  Revert "test(core): added tests to check the configuration affects Java projects"
  feat(configuration): added configuration options for java projects
  refactor(configuration): renamed variable for readability
  doc(core): updated Javadoc to properly detail the effects on Java projects
  test(core): added tests to check the configuration affects Java projects
  test(core): changed test to directly use licenseReport extension
  feat(configuration): renamed configuration
* feature/enable-configuration:
  refactor(core): renamed 'AndroidLicenseReportOptions' to 'LicenseReportOptions'
* feature/enable-configuration:
  doc(readme): wording changes in cofiguration example
* feature/enable-configuration:
  doc(readme): fixed incorrect closure name
@jack-bradshaw
Copy link
Contributor Author

jack-bradshaw commented Jan 22, 2018

I've made some changes to the PR.

  • The tests now directly use the extension name.
  • I've renamed the extension to 'licenseReport'.
  • I've enabled configuration of Java projects in addition to Android projects.
  • Javadoc updates.
  • Added info to the README.

Let me know about the README placement. I wasn't sure where the best place was.

@jaredsburrows
Copy link
Owner

Released 0.8.0 and so this will be in the 0.8.1-SNAPSHOT.

@jaredsburrows jaredsburrows merged commit 7bdcbcc into jaredsburrows:master Jan 22, 2018
@jaredsburrows
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants