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.

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

Implement composite rules, joined via and() #128

Merged
merged 2 commits into from
Dec 16, 2018

Conversation

bogsi17
Copy link
Collaborator

@bogsi17 bogsi17 commented Dec 2, 2018

No description provided.

@codecholeric
Copy link
Collaborator

Thanks a lot 😃 Looks generally good, I've added some minor comments. Also as you can see, the tests have failed. These are ArchUnit's own ArchUnit tests 😉 that want that you add @PublicAPI to public methods you want to expose (it's mainly a check to remind developers to really think carefully about exposed API that we have to support for a long time). So if it's @PublicAPI(usage = ACCESS), public classes and methods should also be final (since the API is not intended for inheritance).

Also it would be great if you could check out https://github.com/TNG/ArchUnit/blob/master/CONTRIBUTING.md
In particular it's necessary that you sign off your commit (check 'sign-off' in IntelliJ or commit via git commit -s from the command line). Referencing the issue would be great, too!
Anyway, thanks a lot for contributing, I really appreciate it!! So I hope all those nagging little comments don't discourage you 😉

@codecholeric
Copy link
Collaborator

PS: Rebasing would be awesome, too 😉 I generally like a linear history where possible...

@bogsi17
Copy link
Collaborator Author

bogsi17 commented Dec 4, 2018

Hi, thanks for the feedback! Where can I find your comments? Can't see them in the 'Files Changed' section.

And yeah, I really should have read this document before :D

@codecholeric
Copy link
Collaborator

Ah sorry, I forgot to click "submit review" 😳

@bogsi17 bogsi17 force-pushed the compositeRules branch 2 times, most recently from 1051e3b to ab6f1cd Compare December 10, 2018 23:28
Moritz Bogs and others added 2 commits December 16, 2018 13:29
Multiple ArchRules can be checked at once via CompositeArchRule.of(rule1).and(rule2). Also added sample tests for both plain, junit4 and junit5

Issue: TNG#78
Signed-off-by: Moritz Bogs <moritz.bogs@tngtech.com>
Issue: TNG#78
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
@codecholeric
Copy link
Collaborator

I've added support for priorities to CompositeArchRule, other than that, it looks good now 😄
Gonna merge it...

@codecholeric codecholeric merged commit 982b68d into TNG:master Dec 16, 2018
codecholeric added a commit that referenced this pull request Feb 21, 2021
Implement composite rules, joined via and()
hankem added a commit that referenced this pull request Nov 25, 2022
Bumps [jemoji](https://github.com/jekyll/jemoji) from 0.12.0 to 0.13.0.

# Changelog

Sourced from [jemoji's changelog](https://github.com/jekyll/jemoji/blob/master/History.markdown).

> ## v0.13.0 / 2022-11-19
> --------------------
> 
> ### Development Fixes
> 
> *   Add Actions CI workflow ([#127](https://github-redirect.dependabot.com/jekyll/jemoji/issues/127))
> *   Add release workflow ([#128](https://github-redirect.dependabot.com/jekyll/jemoji/issues/128))
> *   Create dependabot config for rubygems upgrades ([#126](https://github-redirect.dependabot.com/jekyll/jemoji/issues/126))
> *   Add github-actions to dependabot ([#132](https://github-redirect.dependabot.com/jekyll/jemoji/issues/132))
> *   Bump actions/checkout from 2 to 3 ([#133](https://github-redirect.dependabot.com/jekyll/jemoji/issues/133))
> *   Upgrade Rubocop target version to 2.7 ([#134](https://github-redirect.dependabot.com/jekyll/jemoji/issues/134))
> *   Update rake requirement from ~> 12.0 to ~> 13.0 ([#130](https://github-redirect.dependabot.com/jekyll/jemoji/issues/130))
> 
> ### Minor Enhancements
> 
> *   Update gemoji requirement from ~> 3.0 to >= 3, < 5 & test with both major versions ([#131](https://github-redirect.dependabot.com/jekyll/jemoji/issues/131))

# Commits

*   [`1443020`](jekyll/jemoji@1443020) Merge pull request [#135](https://github-redirect.dependabot.com/jekyll/jemoji/issues/135) from jekyll/release-1-0-0
*   [`6934e3d`](jekyll/jemoji@6934e3d) Release jemoji v0.13.0
*   [`d766153`](jekyll/jemoji@d766153) Update history to reflect merge of [#131](https://github-redirect.dependabot.com/jekyll/jemoji/issues/131) \[ci skip\]
*   [`b251fd9`](jekyll/jemoji@b251fd9) Update gemoji requirement from ~> 3.0 to >= 3, < 5 & test with both major ver...
*   [`663823a`](jekyll/jemoji@663823a) Update history to reflect merge of [#130](https://github-redirect.dependabot.com/jekyll/jemoji/issues/130) \[ci skip\]
*   [`88f650c`](jekyll/jemoji@88f650c) Update rake requirement from ~> 12.0 to ~> 13.0 ([#130](https://github-redirect.dependabot.com/jekyll/jemoji/issues/130))
*   [`d69cb8c`](jekyll/jemoji@d69cb8c) Update history to reflect merge of [#134](https://github-redirect.dependabot.com/jekyll/jemoji/issues/134) \[ci skip\]
*   [`e0e5701`](jekyll/jemoji@e0e5701) Upgrade Rubocop target version to 2.7 ([#134](https://github-redirect.dependabot.com/jekyll/jemoji/issues/134))
*   [`e01a128`](jekyll/jemoji@e01a128) Update history to reflect merge of [#133](https://github-redirect.dependabot.com/jekyll/jemoji/issues/133) \[ci skip\]
*   [`064f142`](jekyll/jemoji@064f142) Bump actions/checkout from 2 to 3 ([#133](https://github-redirect.dependabot.com/jekyll/jemoji/issues/133))
*   Additional commits viewable in [compare view](jekyll/jemoji@v0.12.0...v0.13.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants