-
-
Notifications
You must be signed in to change notification settings - Fork 488
Added v24.0.3.1 to wgpu-native #8467
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
Conversation
Summary of ChangesHello @hardcore-thinking, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily aims to update the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request adds versions v24.0.3+1 and v27.0.2+0 to the wgpu-native package for all supported platforms. The changes are correct and maintain the descending order of versions. However, the PR title and description are slightly misleading as they only mention the addition of v24.0.3+1 (with a typo in the title as v24.0.3.1), while v27.0.2+0 is also added. It would be good to update them to reflect the full scope of changes. Additionally, I've added a comment regarding the maintainability of the xmake.lua file. The current structure requires duplicating version additions across multiple platform blocks, which is error-prone. A refactoring to a more data-driven approach would make future updates much easier.
| add_versions("v27.0.2+0", "0df0231284cd9ab59dfc382f435caf1275f4372ae0635ef1261d6fe1d58c5a0e") | ||
| add_versions("v25.0.2+1", "e0cdcec5e7601117bb0e457e55d1729199d6f203857c72b432b406493c7434f7") | ||
| add_versions("v24.0.3+1", "72d3abf74e0b55f9474602e2ce7f20fd0f9b9c46be8405b45a697ec1ee343436") | ||
| add_versions("v24.0.0+1", "7dff003da706bc413e514a0395ef369f2935a4dc7c99f61cd97e8fee601c9f8e") | ||
| add_versions("v0.19.4+1", "9e1591d60c2d2ee20d6d4a63bc01c7c5eecf7734761673160aa639e550a1ba4d") | ||
| add_versions("v0.17.0+2", "1b8ae05bb7626e037ab7088f9f11fc8bb8341a32800d33857c09ff2fb1b3893f") |
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.
This file has a lot of repeated add_versions blocks for different platforms. Adding a new version, as done in this PR, requires manually adding lines in 8 different places. This is tedious and error-prone.
To improve maintainability, consider refactoring this to be more data-driven. You could define all versions and their platform-specific hashes in a central table at the top of the file, and then iterate through this table in each platform block.
For example:
-- Define all versions and hashes in one place, in descending order
local versions_with_hashes = {
{ version = "v27.0.2+0", hashes = {
["windows-x64"] = "0df0231284cd9ab59dfc382f435caf1275f4372ae0635ef1261d6fe1d58c5a0e",
["windows-x86"] = "542fe3a3e88e617acad2f34d129f0203132b6b175af015cba977dfbee75ccbff",
-- ... other platforms
}},
{ version = "v25.0.2+1", hashes = { ... } },
-- ... other versions
}
-- Helper function to add versions for a specific platform
function add_pkg_versions(plat, arch)
local key = plat .. "-" .. arch -- Or another mapping that fits your needs
for _, v_info in ipairs(versions_with_hashes) do
if v_info.hashes[key] then
add_versions(v_info.version, v_info.hashes[key])
end
end
end
-- Then, in each platform block, call the helper
if is_plat("windows") and is_arch("x64") then
add_urls("...")
add_pkg_versions("windows", "x64")
endThis would centralize version management, making future updates much simpler and safer.
Added
add_versions("v24.0.3+1", ...)for each already supported platforms for the wgpu-native package