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

Fix #79: Some internal refactorings #88

Merged
merged 3 commits into from May 7, 2019

Conversation

Projects
None yet
2 participants
@tlinkowski
Copy link
Contributor

commented Apr 14, 2019

Fixes #79.

1. Clean-up of test projects

  • bumped Gradle to 5.3.1 in SmokeTest
  • aligned all projects (especially test-project and test-project-kotlin) as much as I could
    • forgive my use of regions in build.gradle.kts, but I felt they help there
  • used Matching repositories to dependencies (incubating since 5.1) in local_maven_settings.gradle and smoke_test_settings.gradle
    • note: local builds do not select the latest version automatically now (perhaps it's possible to be configured using dependency resolution, but it didn't seem so easy)
  • javadoc in test-project-kotlin reports an error: "No public or protected classes found to document"
    • it seems to me it should work — when there's only module-info.java, there is a file to document (will investigate another time, though)
  • added gitignore entries for Gradle wrappers in test projects (I opened them as separate projects for IDE support)

2. Refactoring (incl. applying JavaProjectHelper)

The changes turned out to be quite extensive here.

In addition to applying JavaProjectHelper, I refactored heavily. I tried to make the changes as transparent as possible, but I'm afraid they may still be hard to follow (hope it won't be a review blocker).

I'm really pleased with the end result, though (and hope you'll like it too). I aimed at:

  • deduplication
    • added PatchModuleResolver, PatchModuleExtension.getUnpatchedClasspathAsPath
    • added more methods to JavaProjectHelper
  • consistency
    • added AbstractModulePluginTask as a base class for all "non-Gradle" task classes
    • used idioms like:
      • String moduleName = helper().moduleName();
      • var patchModuleExtension = helper().extension(PatchModuleExtension.class);
      • helper().findTask(%name%, %type%).ifPresent(%configure%);
  • reducing control statement nesting (= indentation)
    • extracted configureAfterEvaluate, configureStartScriptsDoFirst, configureStartScriptsDoLast, etc.
    • refactored TestTask.getPackages
  • smaller and more meaningful methods ("hiding how with what")
    • extracted buildCompilerArgs, buildJavaExecJvmArgs, updateRelativePath, buildAddReadsStream, buildAddOpensStream
  • responsibility separation
    • extracted AbstractExecutionMutator and StartScriptsMutator from RunTaskMutator

Of course, I'm open to any questions, remarks, and requests for changes you might have.

As before, I believe the commits are best merged unsquashed.

@tlinkowski tlinkowski marked this pull request as ready for review Apr 18, 2019

@tlinkowski tlinkowski force-pushed the tlinkowski:issue-79 branch from 95e7a9c to bc37dda Apr 18, 2019

@paulbakker paulbakker merged commit 8d84487 into java9-modularity:master May 7, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@tlinkowski tlinkowski deleted the tlinkowski:issue-79 branch May 7, 2019

@tlinkowski tlinkowski restored the tlinkowski:issue-79 branch May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.