-
Notifications
You must be signed in to change notification settings - Fork 97
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
Add enableLoggingFail option for failed introspect to FixtureMonkeyOptions #1069
Add enableLoggingFail option for failed introspect to FixtureMonkeyOptions #1069
Conversation
pop.con seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
@@ -60,6 +60,7 @@ public final class ArbitraryGeneratorContext implements Traceable { | |||
private final int generateUniqueMaxTries; | |||
private final AtomicReference<CombinableArbitrary<?>> generated = | |||
new AtomicReference<>(CombinableArbitrary.NOT_GENERATED); | |||
private final boolean enableLoggingFail; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent the breaking change, could you make a class ArbitraryGeneratorLoggingContext
that includes enableLoggingFail
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public final class ArbitraryGeneratorLoggingContext {
private final boolean enableLoggingFail;
...
public boolean isEnableLoggingFail(){
return enableLoggingFail;
}
}
public final class ArbitraryGeneratorContext implements Traceable {
...
private final ArbitraryGeneratorLoggingContext loggingContext;
...
public ArbitraryGeneratorLoggingContext getLoggingContext() {
return this.loggingContext;
}
...
}
public final class BeanArbitraryIntrospector implements ArbitraryIntrospector {
...
@Override
public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context) {
ArbitraryGeneratorLoggingContext loggingCotext = context.getLoggingContext();
...
if(loggingContext.isEnableLoggingFail()){
...
}
}
...
}
Like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, indeed
public boolean isEnableLoggingFail() { | ||
return enableLoggingFail; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be better to replace it with ArbitraryGeneratorLoggingContext
.
…rbitraryGeneratorLoggingContext (naver#1066)
2715458
to
f8992b1
Compare
@@ -0,0 +1,14 @@ | |||
package com.navercorp.fixturemonkey.api.generator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A license is needed.
/*
* Fixture Monkey
*
* Copyright (c) 2021-present NAVER Corp.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.navercorp.fixturemonkey.api.generator; | ||
|
||
|
||
public final class ArbitraryGeneratorLoggingContext { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the annotation.
@API(since = "1.0.28", status = Status.EXPERIMENTAL)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Summary
add enableLoggingFail option for failed introspect warning log.
#1066
(Optional): Description
||
due to overlapping parts with the existingenableLoggingFail
option in theFailoverIntrospector
.How Has This Been Tested?
Is the Document updated?
enableLoggingFail
option, so I have omitted it for now.