Skip to content

Conversation

@gschueler
Copy link
Member

add existence check operator ~~

@gschueler gschueler requested a review from Copilot April 8, 2025 23:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • src/test/groovy/org/rundeck/plugins/nodes/attributes/AttributeNodeEnhancerSpec.groovy: Language not supported
Comments suppressed due to low confidence (2)

src/main/java/org/rundeck/plugins/nodes/attributes/AttributeNodeEnhancer.java:119

  • The IllegalArgumentException thrown here lacks a descriptive message, which may hinder debugging. Consider adding a message detailing the cause, possibly including the IOException message.
throw new IllegalArgumentException();

src/main/java/org/rundeck/plugins/nodes/attributes/AttributeNodeEnhancer.java:84

  • [nitpick] The parameter name 'addTags1' is unclear and inconsistent with similar naming elsewhere. Consider renaming it to 'addTags' to improve clarity.
public static void addAllTags(final Set<String> tags, Map<String, String> attributes, String addTags1, boolean enableSubstitution) {

@gschueler gschueler requested a review from Copilot April 9, 2025 00:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • src/test/groovy/org/rundeck/plugins/nodes/attributes/AttributeNodeEnhancerSpec.groovy: Language not supported
Comments suppressed due to low confidence (1)

src/main/java/org/rundeck/plugins/nodes/attributes/AttributeNodeEnhancer.java:133

  • Add unit tests to cover the substitution logic, including cases with special characters and missing attribute values to ensure the new functionality behaves as expected.
private static String substitute(Map<String, String> attributes, String value) {

while (matcher.find()) {
String name = matcher.group("name");
String replacement = attributes.get(name);
matcher.appendReplacement(sb, Objects.requireNonNullElse(replacement, ""));
Copy link

Copilot AI Apr 9, 2025

Choose a reason for hiding this comment

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

Use Matcher.quoteReplacement on the replacement string to ensure that any special characters are escaped properly. For example, replace with matcher.appendReplacement(sb, Matcher.quoteReplacement(Objects.requireNonNullElse(replacement, ""))).

Suggested change
matcher.appendReplacement(sb, Objects.requireNonNullElse(replacement, ""));
matcher.appendReplacement(sb, Matcher.quoteReplacement(Objects.requireNonNullElse(replacement, "")));

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants