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

Update Range Check regexp #237

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Update Range Check regexp #237

merged 1 commit into from
Jan 25, 2024

Conversation

Tommi2Day
Copy link

fix #235

@Tommi2Day Tommi2Day changed the title validate ParseRangeString return value in EvaluateThreshold Update Range Check regexp Jan 16, 2024
@atc0005 atc0005 self-requested a review January 17, 2024 12:35
@atc0005 atc0005 added this to the Next Release milestone Jan 17, 2024
@atc0005 atc0005 self-assigned this Jan 17, 2024
@atc0005 atc0005 added bug Something isn't working linting range labels Jan 17, 2024
@atc0005
Copy link
Owner

atc0005 commented Jan 17, 2024

@Tommi2Day

Thanks for submitting this PR.

@infraweavers

As noted on GH-235, this set of changes comes from linting recommendations provided by the GoLand IDE.

Two warnings are present (repeated as applicable):

  • unnecessary non-capturing group
  • redundant character escape

Do you have any concerns with applying the GoLand recommendations?

@atc0005
Copy link
Owner

atc0005 commented Jan 18, 2024

@Tommi2Day We can give this until early next week to give @infraweavers a chance to respond and plan to merge either way then.

If there are any issues that we aren't catching we can always back out the change later and document the specific reasoning behind a need for the earlier regex formats.

@atc0005
Copy link
Owner

atc0005 commented Jan 24, 2024

@Tommi2Day Since we've not heard any objections/corrections from @infraweavers I believe we can proceed with merging these changes.

Are you willing to rebase your PR branch? If not I can do so before the merge.

Thanks.

@Tommi2Day
Copy link
Author

branch has been rebased with current master

@atc0005 atc0005 merged commit 85157e2 into atc0005:master Jan 25, 2024
28 of 30 checks passed
@Tommi2Day Tommi2Day deleted the fix/issue235 branch January 25, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linting range
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RegexpWarnings in Goland
2 participants