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.

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

Comment most similar issue even if similarity threshold not met #57

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

dannyl1u
Copy link
Owner

No description provided.

Copy link

Based on the provided pull request details, here are some potential issues or downsides:

  • The PR introduces a new logic for handling similar issues, where a comment is left even if the similarity threshold is not met (i.e., similarity_issue["distance"] < 1 - (SIMILARITY_THRESHOLD * 0.5)). This might lead to comments being left on issues that don't actually meet the original criteria for closing due to high similarity.
  • The new logic also changes the comment text based on the similarity threshold, which could potentially cause confusion or misleading information in issue trackers.
  • There's no clear indication of why the re module was removed and replaced with a simpler string manipulation (i.e., full_issue = f"{issue_title} {issue_body}"). This change might have unintended consequences if it affects other parts of the codebase.
  • The commit message doesn't mention any changes or updates to existing issues, which could be relevant for maintainers tracking issue history.

Before merging this PR, the author should consider these points:

Have you considered the potential impact of leaving comments on issues that don't meet the original similarity threshold criteria? How will you ensure that this change won't cause confusion or lead to incorrect conclusions?

Also, can you provide more context or reasoning behind removing the re module and replacing it with a simpler string manipulation? Are there any performance or security implications we should be aware of?

Lastly, are there any existing issues or tasks that might be affected by these changes?

@dannyl1u dannyl1u merged commit 6748c20 into main Nov 22, 2024
7 checks passed
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.

1 participant