Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

PROC-1334: Add library support for rule filtering #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

josuf107
Copy link

Copy link
Contributor

@vanhaxl vanhaxl left a comment

Choose a reason for hiding this comment

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

LGTM. We might need to merge this MR #140 firstly to fix the build

import java.util.Set;
import java.util.Stack;

class NodeHunter implements NodeVisitor {
Copy link
Contributor

@stevenchen-indeed stevenchen-indeed Oct 10, 2023

Choose a reason for hiding this comment

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

can we add some javadoc in this class?

Copy link
Author

Choose a reason for hiding this comment

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

Okay added some explanation.

@josuf107
Copy link
Author

Could someone help merge this? @vanhaxl it looks like #140 is merged now

@@ -80,4 +80,44 @@ public static boolean versionInRange(
}
return inRange(version, start, end);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Given all those code will only be used outside the proctor library. https://indeed-pte.slack.com/archives/C1F00STA9/p1688590802320449?thread_ts=1688582115.448299&cid=C1F00STA9

Can we add java doc to each of those methods to explain it is not meant to be called inside proctor lib?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, can we create a subclass and only the subclass has those extra methods?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants