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

numeric comparisons for versions #20383

Closed
wants to merge 3 commits into from
Closed

numeric comparisons for versions #20383

wants to merge 3 commits into from

Conversation

brenthc
Copy link
Contributor

@brenthc brenthc commented Jul 30, 2021

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #21045.

Not sure about adding tests for this, as it'd be necessary to create more than 9 versions so that numeric vs lexical sorting can be verified.

@github-actions github-actions bot added size/S Managed by automation to categorize the size of a PR. service/lexmodels Issues and PRs that pertain to the lexmodels service. needs-triage Waiting for first response or review from a maintainer. labels Jul 30, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @brenthc 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@breathingdust breathingdust added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 31, 2021
@ewbankkit ewbankkit self-assigned this Oct 4, 2021
@ewbankkit ewbankkit added bug Addresses a defect in current functionality. and removed enhancement Requests to existing resources that expand the functionality or scope. labels Oct 4, 2021
@thornleyk
Copy link
Contributor

thornleyk commented Oct 5, 2021

@ewbankkit Hi I'm not sure if this 100% working on a non BUILD lex bot which has only $LATEST and no version. I see nowhere where $LATEST will be returned. This code has a significant issue as the old code could have returned > 1 or $LATEST. this code on the other hand now returns > 0 which would not be expected by existing state. You need something like I have in my closed PR like this

if version > 0 {
returnVersion := strconv.Itoa(version)
return returnVersion, nil
}
return LexBotVersionLatest, nil

ewbankkit added a commit to abebars/terraform-provider-aws that referenced this pull request Oct 5, 2021
@ewbankkit
Copy link
Contributor

@thornleyk Thanks - Yes I did correct that in the merged version.

@brenthc Thanks for the contribution 🎉 👏.

I have incorporated these changes into #21122.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/lexmodels Issues and PRs that pertain to the lexmodels service. size/S Managed by automation to categorize the size of a PR.
Projects
None yet
4 participants