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

wrap min and max #741

Merged
merged 4 commits into from
Jun 9, 2022
Merged

wrap min and max #741

merged 4 commits into from
Jun 9, 2022

Conversation

phlptp
Copy link
Collaborator

@phlptp phlptp commented Jun 7, 2022

in parenthesis for Visual studio 2017 and earlier compatibility.
This fixes some issues from a max/min macro on some earlier Visual studio builds

@phlptp phlptp added the bug label Jun 7, 2022
@phlptp phlptp requested a review from henryiii June 7, 2022 12:42
@henryiii
Copy link
Collaborator

henryiii commented Jun 8, 2022

Any way to test/enforce this not being reintroduced? Maybe a pre-commit pygrep looking for min() or max()?

@henryiii
Copy link
Collaborator

henryiii commented Jun 8, 2022

Maybe:

     - id: avoid-msvc-macro
       name: Avoid MSVC <=2017 min/max macro (use extra parens)
       entry: \b(min|max)\(\)
       language: pygrep
       exclude: .pre-commit-config.yaml

@phlptp
Copy link
Collaborator Author

phlptp commented Jun 9, 2022

I think the pre-commit is working, caught a few more in the tests though I don't think those would ever have been an issue.

Also looks like we need to update the windows CI builds on azure as one of the images currently in use is being deprecated

@henryiii
Copy link
Collaborator

henryiii commented Jun 9, 2022

We'll be out of the brownout later / tomorrow and then we can get #742 in while we still have MSVC 2015. After that, we'll probably have to drop MSVC 2015 support.

@henryiii henryiii closed this Jun 9, 2022
@henryiii henryiii reopened this Jun 9, 2022
@henryiii henryiii merged commit 443c1a9 into CLIUtils:main Jun 9, 2022
@github-actions github-actions bot added needs changelog Hasn't been added to the changelog yet needs README Needs to be mentioned in the README labels Jun 9, 2022
@henryiii henryiii removed the needs README Needs to be mentioned in the README label Sep 14, 2022
@henryiii henryiii removed the needs changelog Hasn't been added to the changelog yet label Oct 28, 2022
@phlptp phlptp deleted the max_in_parenthesis branch March 11, 2023 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants