917 feature overriding of rules #992
Conversation
|
Next: add report structure |
|
Todo: groups, constraints and concepts should be able to override more than one of their type |
|
Todo: fix json schema for yamls (bug) and tests |
|
Add Maven IT test for overriding groups, constraints and concepts |
| xmlStreamWriter.writeEndElement(); | ||
| } | ||
| List<String> overriddenIds = group.getOverriddenIds(); | ||
| if (overriddenIds != null && !overriddenIds.isEmpty()) { |
There was a problem hiding this comment.
This code could be extracted to a separate method taking the element name and overridden ids as parameters. The method could then be re-used for concepts and constraints.
| LOGGER.warn("Could not find concepts matching to '{}'.", conceptPattern); | ||
| } else { | ||
| for (Concept matchingConcept : matchingConcepts) { | ||
| if (ruleSet.getConceptBucket() |
There was a problem hiding this comment.
This should be part of the method applyConcept (which is called directly below)
core/rule/src/main/java/com/buschmais/jqassistant/core/rule/api/executor/RuleSetExecutor.java
Outdated
Show resolved
Hide resolved
| .stream() | ||
| .map(Concept.ProvidedConcept::getProvidedConceptId) | ||
| .collect(toList()); | ||
| if (!new HashSet<>(overridingConceptsProvides).equals(new HashSet<>(overriddenConceptsProvides))) { |
There was a problem hiding this comment.
Shouldn't this be overridingConceptsProvides.containsAll(overriddenConceptsProvides) ?
There was a problem hiding this comment.
This was my initial thought too, but the problem I see here: Shouldn't the overriding concepts provide the same concepts as the overridden concepts AND vice versa to ensure reliable behavior? Or should the overriding concept be able to provide more?
No description provided.