Skip to content

Conversation

@shaohuzhang1
Copy link
Contributor

No description provided.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Oct 10, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Oct 10, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@liuruibin liuruibin merged commit f3ef42b into v2 Oct 10, 2025
8 of 13 checks passed
@liuruibin liuruibin deleted the pr@v2@chore_re_find branch October 10, 2025 02:37
if isinstance(pattern, str) and (not pattern or not pattern.strip()):
return []

try:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code has a few minor issues that require adjustments:

  1. The function name re_findall does not match the expected behavior compared to Python's built-in re.findall. This could cause confusion for users.

  2. There is no explicit documentation or comments explaining why certain parts of the function are included or what they do.

  3. The use of None as a condition makes sense for empty patterns but should be commented to clarify its purpose.

  4. The check for an empty string and whitespace-only strings at the beginning of the pattern might be unnecessary and should be removed since it would behave correctly with standard regular expressions.

Here is the revised version with these improvements:

def re_finds_all(pattern, text):
    """
    Find all non-overlapping matches of the pattern in the string using re module.
    
    :param pattern: Regex pattern as a string or compiled regex object
    :param text: String to search within
    :return: List of all matched substrings
    """
    
    if pattern is None:
        return []
    
    try:
        import re
        return re.findall(pattern, text)
    except TypeError as e:
        print(f"Error while finding pattern: {e}")
        return []

# Example usage:
# result = re_finds_all(r'\b\w+\b', "Hello World")
# print(result)  # Output: ['Hello', 'World']

Key Changes:

  • Function Name: Renamed to re_finds_all to clearly indicate its intended functionality.
  • Comments: Added docstring to describe the function and its parameters.
  • Error Handling: Included a basic error handling mechanism for catching type errors when attempting to find the pattern in the text.
  • Removed Unnecessary Check: Removed the additional check for empty or whitespace-only strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants