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 the possibility to define a layer using a DescribedPredicate - Issue 228 #238

Merged
merged 3 commits into from
Oct 20, 2019
Merged

Add the possibility to define a layer using a DescribedPredicate - Issue 228 #238

merged 3 commits into from
Oct 20, 2019

Conversation

mikomatic
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Sep 20, 2019 •
edited by ghost
Loading

DeepCode Report (#736f1a)

DeepCode analyzed this pull request.
There are no new issues.

@mikomatic mikomatic changed the title Issue 228 Add the possibility to define a layer using a DescribedPredicate - Issue 228 Sep 20, 2019
@mikomatic mikomatic force-pushed the issue-228 branch 2 times, most recently from 048fed5 to a18d3e8 Compare September 20, 2019 22:20
@mikomatic
Copy link
Contributor Author

Sorry for all the force pushing :/

This PR should not move anymore and is ready for review.

mikomatic and others added 3 commits October 20, 2019 21:50
@mikomatic @codecholeric
Rename function to convey more accurate checking
7eb5070 
> Signed-off-by: Miguel Ortega <miguel.ortega84@gmail.com>
@mikomatic @codecholeric
add possibility to define layers of layered architecture by predicate…
333896e 
…, not just by package

Issue: TNG#228

> Signed-off-by: Miguel Ortega <miguel.ortega84@gmail.com>
@codecholeric
Review: Extracted some logic to new class LayerDefinitions, since it …
736f1ae 
…felt that too many low level aspects of converting layers to predicates were covered by LayeredArchitecture, i.e. mixing of different abstraction levels. Also replaced copy & paste test with ignores by DataProvider tests for the two essential aspects, reporting violations and the description. The description of predicates was not covered by any test. Moved the quotes "'%s'" to the description of definedBy(packages), since an arbitrary predicate should have the freedom to decide, if it wants to be quoted or not. Also the description "..foo'. '..bar.." without quotes around made no sense, so you had to look in two separate places to understand the connection.
@codecholeric
Copy link
Collaborator

Hey, thanks a lot 😄 Sorry that it took me so long to look into it!!
I took the liberty to adjust your commit messages a little, because I think commit messages should be self-contained and not depend on e.g. GitHub. If you just write "implemented issue #228", then the info what this commit was about is gone as soon as the issue on GitHub is gone. Also if one quickly looks into the history later, it's tedious to look for the number on GitHub, just to find out what the goal of the commit was.
I refactored the code a little, because I had the feeling that LayeredArchitecture did a lot of low level things with the layer definitions and the wrapped predicate.
Anyway, I'm gonna merge it now so it can go right into the upcoming release, thanks again!!

@codecholeric codecholeric merged commit 45a808b into TNG:master Oct 20, 2019
codecholeric added a commit that referenced this pull request Feb 21, 2021
@codecholeric
Merge pull request #238 from mikomatic/issue-228
e1d5cc2 
Add the possibility to define a layer using a DescribedPredicate

Resolves: #228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mikomatic @codecholeric