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

DROOLS-4425: Refactor drools-workbench-models Attribute constants #2508

Merged
merged 6 commits into from
Oct 7, 2019
Merged

DROOLS-4425: Refactor drools-workbench-models Attribute constants #2508

merged 6 commits into from
Oct 7, 2019

Conversation

jomarko
Copy link
Contributor

@jomarko jomarko commented Aug 19, 2019 •
edited
Loading

@jomarko jomarko mentioned this pull request Aug 19, 2019
Merged
@jomarko jomarko force-pushed the DROOLS-4425-Attribute branch from 130cd28 to 4a5f124 Compare August 19, 2019 14:43
@jomarko jomarko force-pushed the DROOLS-4425-Attribute branch from 4a5f124 to cfd6574 Compare August 22, 2019 13:45
@jomarko jomarko force-pushed the DROOLS-4425-Attribute branch 5 times, most recently from 309dac8 to 9dec438 Compare October 2, 2019 09:06
@jomarko jomarko marked this pull request as ready for review October 2, 2019 09:06
@jomarko jomarko requested review from Rikkola and dupliaka October 2, 2019 09:06
dupliaka
dupliaka reviewed Oct 2, 2019
View reviewed changes
...models-datamodel-api/src/main/java/org/drools/workbench/models/datamodel/rule/Attribute.java
import java.util.stream.Stream;

import org.kie.soup.project.datamodel.oracle.DataType;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Can we add some comment why do we need this attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dupliaka thank you, I added a javadoc in a new commit

Rikkola
Rikkola reviewed Oct 2, 2019
View reviewed changes
...ble/src/main/java/org/drools/workbench/models/guided/dtable/shared/model/AttributeCol52.java Outdated
@@ -17,6 +17,8 @@

import java.util.List;

import org.drools.workbench.models.datamodel.rule.Attribute;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, done

Rikkola
Rikkola reviewed Oct 2, 2019
View reviewed changes
...java/org/drools/workbench/models/guided/dtable/shared/validation/DecisionTableValidator.java Outdated
@@ -17,6 +17,7 @@

import java.util.Set;

import org.drools.workbench.models.datamodel.rule.Attribute;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, done

Rikkola
Rikkola approved these changes Oct 2, 2019
View reviewed changes
Copy link
Contributor

@Rikkola Rikkola left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments. And they were fixed.

dupliaka
dupliaka reviewed Oct 2, 2019
View reviewed changes
...ls-datamodel-api/src/main/java/org/drools/workbench/models/datamodel/rule/RuleAttribute.java Outdated
@@ -45,26 +45,26 @@ public RuleAttribute() {
public String toString() {
StringBuilder ret = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be final

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix in new commit, thank you

dupliaka
dupliaka reviewed Oct 2, 2019
View reviewed changes
...ls-datamodel-api/src/main/java/org/drools/workbench/models/datamodel/rule/RuleAttribute.java Outdated
@@ -45,26 +45,26 @@ public RuleAttribute() {
public String toString() {
StringBuilder ret = new StringBuilder();
ret.append( this.attributeName );
if ( NOLOOP.equals( attributeName ) ) {
if ( NO_LOOP.getAttributeName().equals( attributeName ) ) {
ret.append( " " );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the regression, but I suggest to avoid concatenating characters as strings in StringBuffer/StringBuilder.append methods. Use ' ' instead of " "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do in new commit for all occurrences in this class, thank you

dupliaka
dupliaka reviewed Oct 2, 2019
View reviewed changes
.../main/java/org/drools/workbench/models/guided/dtable/shared/model/GuidedDecisionTable52.java Outdated
@@ -28,6 +28,7 @@
import org.drools.workbench.models.guided.dtable.shared.model.adaptors.FactPatternPattern52Adaptor;
import org.kie.soup.project.datamodel.imports.HasImports;
import org.kie.soup.project.datamodel.imports.Imports;
import org.kie.soup.project.datamodel.oracle.DataType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused import

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will remove, thank you

dupliaka
dupliaka reviewed Oct 2, 2019
View reviewed changes
...s/workbench/models/guided/dtable/shared/validation/DecisionTableValidatorAttributesTest.java Outdated
GuidedDecisionTable52.RULEFLOW_GROUP_ATTR,
GuidedDecisionTable52.DIALECT_ATTR,
GuidedDecisionTable52.NEGATE_RULE_ATTR );
return Stream.of(Attribute.values()).map(a->a.getAttributeName()).collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lambda could be replaced by the method reference Attribute::getAttributeName

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in new commit, thank you

dupliaka
dupliaka approved these changes Oct 2, 2019
View reviewed changes
Copy link
Contributor

@dupliaka dupliaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve with suggestions

dupliaka
dupliaka approved these changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@dupliaka dupliaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with suggestions

yesamer
yesamer approved these changes Oct 4, 2019
View reviewed changes
Copy link
Contributor

@yesamer yesamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, just 2 unused imports to remove.

...in/java/org/drools/workbench/models/guided/dtable/shared/validation/HitPolicyValidation.java Outdated
@@ -20,20 +20,24 @@
import java.util.HashSet;
import java.util.Set;

import org.drools.workbench.models.datamodel.rule.Attribute;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jomarko This is an unused import

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, thank you

...rg/drools/workbench/models/guided/dtable/backend/util/GuidedDecisionTableUpgradeHelper2.java Outdated
import java.util.List;
import java.util.Objects;

import org.drools.workbench.models.datamodel.rule.Attribute;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jomarko Another unused import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, thank you

@jomarko jomarko force-pushed the DROOLS-4425-Attribute branch from 308f931 to ac83d76 Compare October 4, 2019 08:39
@jomarko
Copy link
Contributor Author

jomarko commented Oct 7, 2019

@kiegroup/gatekeepers could we please merge?

@danielezonca danielezonca merged commit 0553145 into apache:master Oct 7, 2019
tarilabs pushed a commit to tarilabs/drools that referenced this pull request Jun 28, 2022
@hmanwani-rh
BXMSDOC-5870: Added Boxed List expression section and updated Decisio…
5f2867f 
…n navigator view screenshot (apache#2508)
nprentza pushed a commit to nprentza/drools that referenced this pull request Jul 15, 2022
@hmanwani-rh
BXMSDOC-5870: Added Boxed List expression section and updated Decisio…
0003af4 
…n navigator view screenshot (apache#2508)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rikkola Rikkola Rikkola approved these changes

@yesamer yesamer yesamer approved these changes

@dupliaka dupliaka dupliaka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jomarko @Rikkola @yesamer @dupliaka @danielezonca