-
Notifications
You must be signed in to change notification settings - Fork 7
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
#1: Added Layer rules L1-L12 #10
Conversation
LayerDependencyRulesTest.java includes all necessary tests.
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.
@vlad961 Thanks for your PR. Nice that you found the solution to prevent errors if a layer is empty. 👍
I have added several review comments. Can you please have a look and try to address my concerns?
.layer("common").definedBy("com.devonfw.sample.archunit.common..") // | ||
.layer("logic").definedBy("com.devonfw.sample.archunit.logic..") // | ||
.layer("dataaccess").definedBy("com.devonfw.sample.archunit.dataaccess..") // | ||
.layer("service").definedBy("com.devonfw.sample.archunit.service..") // | ||
.layer("client").definedBy("com.devonfw.sample.archunit.client..") |
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.
@vlad961 the package root and especially the component should not be encoded here:
- We might have multiple components in the same application and each could have any up to all out of this layers
- As we want to provide a "starting point" and "template" for projects using devonfw and arch-unit we should not tie this to
com.devonfw.sample.archunit
as otherwise every project would have to adopt this to their own root package what is however not neccessary.
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 have changed each layer definition by exchanging the prefix: "com.devonfw.sample.archunit" with a second "." (dot).
src/test/java/com/devonfw/sample/archunit/ArchitectureTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/devonfw/sample/archunit/LayerDependencyRulesTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/devonfw/sample/archunit/LayerDependencyRulesTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/devonfw/sample/archunit/LayerDependencyRulesTest.java
Outdated
Show resolved
Hide resolved
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.
@vlad961 thanks for your update. Looks almost perfect now. 👍
Still I have two review annotations you should address.
src/test/java/com/devonfw/sample/archunit/ArchitectureTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/devonfw/sample/archunit/ArchitectureTest.java
Outdated
Show resolved
Hide resolved
Postponed its implementation as discussed.
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.
@vlad961 thanks for your rework. Ready for merge. 👍
#1
Implemented Layer rules L1-12 with Archunit