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.

Sign up for GitHub

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

Jump to bottom

Add ensureAllClassesAreContainedInLayers() to Architectures #278

Merged
merged 2 commits into from
Jul 2, 2022
Merged

Add ensureAllClassesAreContainedInLayers() to Architectures #278

merged 2 commits into from
Jul 2, 2022
+276 −5

Conversation

roxspring
Copy link
Contributor

@roxspring roxspring commented Dec 14, 2019 •
edited by codecholeric
Loading

This will add possibilities to ensure that all classes under test are contained within the respective LayeredArchitecture/OnionArchitecture. It will help users to make sure they don't overlook any classes when defining their architectures. Furthermore, it will help with maintainability of the architectures when new classes are added to the code base later on. In detail the following methods have been added to LayeredArchitecture and OnionArchitecture:

  • ensureAllClassesAreContainedInArchitecture()
  • ensureAllClassesAreContainedInArchitectureIgnoring(packageIdentifiers)
  • ensureAllClassesAreContainedInArchitectureIgnoring(predicate)

Resolves #222

@ghost
Copy link

ghost commented Dec 14, 2019 •
edited by ghost
Loading

Congratulations 🎉. DeepCode analyzed your code in 2.011 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard

@roxspring roxspring force-pushed the feature/gh-222-all-classes-are-covered branch from 1bbe5c9 to 58ff904 Compare December 14, 2019 01:22
@roxspring
Copy link
Contributor Author

@codecholeric can you take a look at this and let me know how it compares to what you imagined? I'm fairly new to ArchUnit and wanted to double check the direction before going much further.

@codecholeric
Copy link
Collaborator

Thanks a lot for looking into this 😃 I've added a couple of thoughts, in particular I think it is important that we compare this to #273, since this feels to me like two sides of the same coin, don't you think? Both deal with a kind of configuration of how I want to test the layered architecture.
Do I want to allow empty layers when I test it? Do I want to allow some classes not covered by any layer at all? This does not influence the basic structure of the layered architecture, it's more "meta-information" of how I want to test the supplied set of classes.

@roxspring
Copy link
Contributor Author

Thanks for the review. I agree this relates very closely to #273 and that consistency is important. I'll take a closer look at the code there and try to bring my branch in line with that one.

@codecholeric codecholeric mentioned this pull request Dec 17, 2019
Merged
@roxspring roxspring changed the title Add LayeredArchitecture.whereAllClassesAreCovered() Add LayeredArchitecture.ensureAllClassesAreContainedInLayers() Dec 22, 2019
@roxspring roxspring changed the title Add LayeredArchitecture.ensureAllClassesAreContainedInLayers() Add LayeredArchitecture ensureAllClassesAreContainedInLayers() Dec 22, 2019
@roxspring roxspring force-pushed the feature/gh-222-all-classes-are-covered branch 3 times, most recently from 11fde7d to 3525437 Compare January 10, 2020 23:15
@roxspring roxspring force-pushed the feature/gh-222-all-classes-are-covered branch from 3525437 to 16b2686 Compare January 19, 2020 22:45
@roxspring roxspring force-pushed the feature/gh-222-all-classes-are-covered branch from 16b2686 to 3adc6d3 Compare May 19, 2020 23:22
@clojj
Copy link

clojj commented Feb 10, 2021 •
edited
Loading

Hi, @codecholeric
nice PR... can anything be done here ?

Base automatically changed from master to main February 22, 2021 00:58
@codecholeric codecholeric force-pushed the feature/gh-222-all-classes-are-covered branch from 3adc6d3 to 4c3df05 Compare June 26, 2022 05:56
@codecholeric codecholeric marked this pull request as ready for review June 26, 2022 05:56
@codecholeric
Copy link
Collaborator

Hey, I took the liberty to fix up and finalize this PR, since it seemed a little orphaned to me 😉 Hope that's alright with you!!

@codecholeric codecholeric changed the title Add LayeredArchitecture ensureAllClassesAreContainedInLayers() Add ensureAllClassesAreContainedInLayers() to Architectures Jun 26, 2022
@codecholeric codecholeric changed the title Add ensureAllClassesAreContainedInLayers() to Architectures Add ensureAllClassesAreContainedInLayers() to Architectures Jun 26, 2022
@codecholeric codecholeric changed the title Add ensureAllClassesAreContainedInLayers() to Architectures Add ensureAllClassesAreContainedInLayers() to Architectures Jun 26, 2022
@roxspring
Copy link
Contributor Author

Please feel free! I've changed jobs since starting this, and although I've introduced use of ArchUnit, we don't have a strong layered architecture so this has become a very low priority 😕

@codecholeric
Copy link
Collaborator

No worries, I somehow completely lost track 🙈 But I think this will be a really nice addition that users will appreciate 🙂

roxspring added 2 commits June 30, 2022 14:53
@roxspring @codecholeric
add LayeredArchitecture.ensureAllClassesAreContainedInArchitecture()
ca2ba4d 
This will give users the possibility to ensure that they have not missed any classes of the application when defining a `layeredArchitecture()`.

Signed-off-by: Rob Oxspring <[email protected]>
@roxspring @codecholeric
add OnionArchitecture.ensureAllClassesAreContainedInArchitecture()
3352e33 
This will give users the possibility to ensure that they have not missed any classes of the application when defining an `onionArchitecture()`.

Signed-off-by: Rob Oxspring <[email protected]>
@codecholeric codecholeric force-pushed the feature/gh-222-all-classes-are-covered branch from 4c3df05 to 3352e33 Compare June 30, 2022 07:53
GandalfIX
GandalfIX approved these changes Jul 1, 2022
View reviewed changes
Copy link

@GandalfIX GandalfIX left a comment

Choose a reason for hiding this comment

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

This is a nice and simple addition which will help in our project as we are right within a transition from layer to onion :-)

@clojj
Copy link

clojj commented Jul 1, 2022

Nice!👌

@codecholeric codecholeric merged commit 65843a8 into TNG:main Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codecholeric codecholeric codecholeric left review comments

@GandalfIX GandalfIX GandalfIX approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is there a way log/trace files that are not covered by any layer of a LayeredArchitecture?
4 participants
@roxspring @codecholeric @clojj @GandalfIX