Feature api refactoring Issue #45 #78
Conversation
# Conflicts: # deepsampler-provider/src/testFixtures/java/de/ppi/deepsampler/provider/common/SamplerAspectTest.java
+ non-persistent samples are ignored while loading samples from file
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
It would be possible to separate persistent/non-persistent samples more consequently, but it would come with introducing redundancy + a bit overengineering (i guess u would need new record handler + repo + addon-mechanism for external repos. (see PR as approved, comment are meant as ideas to think about, actually I dont know whats better in terms of long time maintainability)
| @@ -16,6 +16,7 @@ | |||
| private List<ParameterMatcher<?>> parameterMatchers = new ArrayList<>(); | |||
| private Answer<Throwable> answer; | |||
| private String sampleId; | |||
| private boolean isMarkedForPersistence; | |||
There was a problem hiding this comment.
I dont know if I like this, it might be better just to create another repository for persistent samples? So samples wont get mixed and you dont have to change core for adjusting persistence features? On the down side, it would introduce more redundancy and it would have been necessary to have a possibility to add external repositories in core, I think that would have some kind of charm.
There was a problem hiding this comment.
I'm using this marker only to find inconsistencies in the Samplers, so that we can throw Exceptions with precise and meaningfull messages.
I also was thinking about introducing a new separated repository. I think this would lead to a cleaner and saver separation. I also like the idea of open extendable repositories. I didn't build something like this, because I think that this would have rather complex consequences and I honestly don't find the time for such big changes right now. Since the flag is only used for validation, I thought that a flag would be enough, at least for now.
Maybe we can reconsider separated repositories in version 3.0.
There was a problem hiding this comment.
BTW: THX for the review 💯
No description provided.