-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Update fmt to 11.2.0 #15159
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
feat: Update fmt to 11.2.0 #15159
Conversation
✅ Deploy Preview for meta-velox canceled.
|
447c104 to
7ddc242
Compare
7ddc242 to
761be5e
Compare
|
Removing the commit with the new dependency images. Run with the new dependency images was all green. |
761be5e to
cb7c927
Compare
| | xsimd | 10.0.0 | Yes | | ||
| | re2 | 2024-07-02 | Yes | | ||
| | fmt | 10.1.1 | Yes | | ||
| | fmt | 11.2.0 | Yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the main CMakeLists we only require version 9.0.0 and later. So this version is not directly enforced.
There was a problem with the previous upgrade. Not a problem in the CI later on when we use a newer CLang because that will use the new version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a comment regarding that though - that we should avoid api's that are not present in 9.0.0 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can enforce it in the cmake. If an older version is found it will be using the new bundled version.
I think the reason that it is still 9.0.0 is for backwards compatibility with additional code (outside of this repo) that uses an older fmt version. Or this isn't necessary anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it will use the newer bundled version ,which is fine, but should point out that code that is not backwards compatible with 9.0.0 risks chance of being reverted (Its unfair on our part to add this additionally on developers , but I believe in most cases they are compatible , so hopefully its not too bad.)
| | xsimd | 10.0.0 | Yes | | ||
| | re2 | 2024-07-02 | Yes | | ||
| | fmt | 10.1.1 | Yes | | ||
| | fmt | 11.2.0 | Yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a comment regarding that though - that we should avoid api's that are not present in 9.0.0 .
6dc2dad to
7507ffe
Compare
7507ffe to
35b0b82
Compare
Summary: Pull Request resolved: facebookincubator#15159 Reviewed By: kKPulla Differential Revision: D84624987 Pulled By: kgpai fbshipit-source-id: 8b95d426b0bce93fc1351e54a4abdcf67d0adf7e
No description provided.