Use QDox 2 to generate sugar class from a single pass of parsing source #66
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Download from Central rather than committing as a binary to avoid growing the repository on upgrade.
Upgrade the build and fix API changes.
- Make all sample matcher methods match the requirements for factory methods. - Move sample matchers into separate source, to work around QDox not considering annotations when parsing from loaded classes. - Assert that an expected item is there when testing.
…Test. To prepare for consolidating into a single reader, copy across all tests.
These are tests for things the reflective reader couldn't do. As QDox provides these, change them to positive tests.
…ective reader. Rely more on QDox 2.0 and ensure that the QDox reader passes all tests that the reflective reader does.
Copy over some structure from the original reader implementation, to clarify behaviour.
For running in builds, don't require clients to create the specified directory in advance.
|
👍 (Maven build) |
|
Looks good, but could you separate the change to use QDox2 from the change to download dependencies from another repo instead of having a self-contained repo? That's a more contentious issue and is therefore likely to delay acceptance of the rest of the changes which are an obvious improvement to the build, so the two changes would be best as two different pull requests |
From: https://repo1.maven.org/maven2/com/thoughtworks/qdox/qdox/2.0-M2/qdox-2.0-M2.jar sha1sum: fa5d16f9a3ab0bba30e37bd0881f32b2402bcb93
|
Fair point; it's an unrelated change, and there's no reason to make it here. I've pushed the binary in this branch and will make a separate PR for the more controversial change. |
npryce
added a commit
that referenced
this pull request
Nov 1, 2014
Use QDox 2 to generate sugar class from a single pass of parsing source
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
hamcrest-generatorcurrently contains two separate reader classes, one which uses reflection and one which uses QDox, which work together to extract all information. This means that the sugar classes need to be generated after compilation, which complicates things when trying to make changes to the build.This change upgrades to a recent milestone of QDox 2 which has good enough generics parsing to cover all the cases that previously required reflection. It throws away
ReflectiveFactoryReaderfor an improvedQDoxFactoryReaderthat covers all the previous cases.All tests are carried across to the new implementation. Now there's a single Reader, they need to be marked as factory methods for all tests and parsed from files rather than taken from the classpath.
The primary motivation for this change is to allow for a Maven build (in a separate build-with-maven branch), where
Matchers.javaandCoreMatchers.javacan be generated in thegenerate-sourcesphase. However, it also simplifies things by shrinking the codebase and moving to a single-pass model.~~In another step inspired by Ivy and Maven, it downloads the new version from Central rather than committing it as a blob. This saves growing the git repo by ~200K every time a new version is used.~~