Skip to content

clang-13 version of cstdlib doesn't define integer types in global na…#5031

Merged
LarryOsterman merged 8 commits intoAzure:mainfrom
LarryOsterman:larryo/fix5007
Oct 17, 2023
Merged

clang-13 version of cstdlib doesn't define integer types in global na…#5031
LarryOsterman merged 8 commits intoAzure:mainfrom
LarryOsterman:larryo/fix5007

Conversation

@LarryOsterman
Copy link
Copy Markdown
Member

…mespace

Fixes #5007

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

  • C++ Guidelines
  • Doxygen docs
  • Unit tests
  • No unwanted commits/changes
  • Descriptive title/description
    • PR is single purpose
    • Related issue listed
  • Comments in source
  • No typos
  • Update changelog
  • Not work-in-progress
  • External references or docs updated
  • Self review of PR done
  • Any breaking changes?

@LarryOsterman
Copy link
Copy Markdown
Member Author

/azp run cpp - core

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Comment thread sdk/core/azure-core/inc/azure/core/base64.hpp Outdated
@LarryOsterman
Copy link
Copy Markdown
Member Author

Ping :).

@ahsonkhan
Copy link
Copy Markdown
Contributor

Anton has suggested (and I agree with him) that we should move all the integer types to use the std::xxx form.

That sounds like a great solution and one we can put in the SDK guidelines.

Comment thread eng/pipelines/templates/stages/platform-matrix.json Outdated
Comment thread eng/pipelines/templates/stages/platform-matrix.json Outdated
Comment thread eng/pipelines/templates/stages/platform-matrix.json Outdated
Comment thread eng/pipelines/templates/stages/platform-matrix.json Outdated
Copy link
Copy Markdown
Member

@danieljurek danieljurek left a comment

Choose a reason for hiding this comment

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

EngSys changes LGTM

Comment thread sdk/core/azure-core/inc/azure/core/uuid.hpp Outdated
Copy link
Copy Markdown
Member

@RickWinter RickWinter left a comment

Choose a reason for hiding this comment

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

Add a comment that we will remove the header in future cleanup. Otherwise LGTM

Copy link
Copy Markdown
Contributor

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

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

Fingers crossed this fixes the customer's vcpkg install issue :)

Comment thread sdk/core/azure-core/inc/azure/core/uuid.hpp Outdated
Comment thread sdk/core/azure-core/inc/azure/core/base64.hpp Outdated
@LarryOsterman LarryOsterman enabled auto-merge (squash) October 17, 2023 18:50
@LarryOsterman LarryOsterman merged commit fbec023 into Azure:main Oct 17, 2023
@LarryOsterman LarryOsterman deleted the larryo/fix5007 branch October 27, 2023 17:14
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.

azure-core build failure with gcc 13

4 participants