Enhance FreezingArchRule #252
Merged
Conversation
This file contains 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
DeepCode Report (#e946a3)DeepCode analyzed this pull request. |
hankem
approved these changes
Oct 22, 2019
archunit/src/main/java/com/tngtech/archunit/library/freeze/ViolationStoreFactory.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/ArchConfigurationTest.java
Outdated
Show resolved
Hide resolved
hankem
approved these changes
Oct 23, 2019
There was a problem hiding this comment.
I've added my suggestions to branch PR#252 of my fork. Feel free to merge them, if you like. 😉
codecholeric
force-pushed
the
enhance-freezing-archrule
branch
from
8b8a3a5 to
f8cc111
Compare
… by passing the key prefixed with "archunit." as system property "-Darchunit.key.in.props=overwritten" Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
… and update Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
…d the possibility to override ArchUnit configuration by system properties. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
codecholeric
force-pushed
the
enhance-freezing-archrule
branch
from
f8cc111 to
e946a3f
Compare
|
Kewl, thank you so much for your support 😃 Do you wanna quickly go over your points and check out, that I've covered everything? |
|
I'll be curious to have a look, but you don't have to wait for me if you're already in the flow of merging. All of my comments were anyways all just cosmetics. |
|
Sure, I'll merge it then 😉 Due to my busy schedule over the last months, I really have some reviewing and merging to do 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.
As mentioned in #211,
FreezingArchRuleneeds some more configuration options to prevent misuse. Furthermore there are different scenarios howFreezingArchRuleis used. In some scenarios, developers are required to update the violation store and check the current state into the repository. In some scenarios the CI job is more sophisticated and clones, updates and pushes a violation store automatically. In any case, within a CI environment it will normally not be necessary to ever create a new violation store, but on the contrary, this will easily be a sign of misconfiguration (the violation store being mounted / checked out to a different folder than configured forFreezingArchRule).This PR will make it configurable, if
FreezingArchRulemay create a new default store, or if updates to the store are allowed in the specific environment.Note that the default for allowing to create a violation store is now
false, i.e. without further configuration running aFreezingArchRulewithout any existing violation store will cause an exception and explicitly demand to configureallowStoreCreation=true.To make it easier to configure these capabilities per environment (e.g. one job or a developer might be allowed to update the violation store, another job may not), ArchUnit configuration may now be overwritten by system properties. This will in particular make it easier to configure the violation store folder based on the specific environment.
Resolves: #211