-
Notifications
You must be signed in to change notification settings - Fork 6
Enforce package hierarchy #39
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
Merged
Merged
Conversation
This file contains hidden or 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
3 tasks
14 tasks
44ed928 to
97684b3
Compare
25 tasks
97684b3 to
4628318
Compare
OpEnvironment returns an OpBuilder, so OpBuilder should be in the same package
Since OpEnvironments have Hints, the Hints need to be in the base package.
This commit also makes some changes to SimplifiedOpRef that make it more correct. I chose to throw UnsupportedOperationException for many of the standard OpRef methods because the concrete type that the SimplifiedOpRef is asking for is rather indeterminate - it is really asking for an Op that is of a set of Types, so to pin a particualar set of arguments, a particular output type, etc. is not possible until you have a SimplifiedOpInfo to narrow down these Types. Similarly, we throw an UnsupportedOperationException for typesMatch because the typing of the SimplifiedOpRef is unclear. We could match against all possible permutations of the input/output types, but we don't really have enough information about how to form the opType. We could probably find a way to devise all opType permutations, but this would be a pretty computationally expensive method to run, so for now we'll just throw an error
This prevents us from having to declare it. There was no reason why we should
These methods do not depend on Ops, so we should export them for broader usage
This allows us to later remove the org.scijava.param fork
This commit also adds an isRequired() field to Member.
a4fd461 to
d4f9fc4
Compare
Forked from de.andrena.tools.nopackagecycles:no-package-cycles-enforcer-rule.
The current situation is untenable since Creators depends on the net.imagej.ops2.create.kernel and net.imagej.ops2.create.img packages. Ideally, we'd split out the image- and kernel create Ops into their own packages like we had, but the kernel and image creator Op fields rely on the (non-static) Op fields in the main package. Therefore if we moved these Ops to the subpackages, we would have to duplicate code. To prevent this duplication, we shall just consolidate to one package.
There seemed to be no reason for this package. The contents did not really deal with Meshes, and many Ops in the net.imagej.ops2.geom.geom3d package (that actually imported Mesh) depended on this subpackage (which is bad). For these reasons, let's just absorb the mesh subpackage into the geom3d package
This was a tough decision, as I'd like to keep the provenance stuff separate from the matcher, but we have to take the OpHistory in the OpWrappers, so we have to make this change. It is probably worth considering making a scijava-ops-provenance module that can stand alone from scijava-ops(-engine)
d4f9fc4 to
90b38f0
Compare
ctrueden
approved these changes
Nov 20, 2021
Member
ctrueden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I released a new scijava-maven-plugin 2.0.0 with the two new package rules, and updated this PR accordingly, such that there are no more WIP commits. LGTM now.
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.
This PR adds a set of package enforcement rules to all incubator builds. As of this writing, we enforce two rules:
TODO:
Closes scijava/scijava#62