Fix sparkle .zip after refactoring #3620
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request updates the logic for identifying the Sparkle ZIP file in GitHub releases. It changes the expected filename from a dynamic {version}-{platform}.zip to a static Omi.zip. This change is consistently applied to both the primary search and the fallback mechanism within the _get_sparkle_zip_download_url function. This suggests an adaptation to a new, simplified naming convention for the Sparkle ZIP asset, which aligns with the pull request title "Fix sparkle .zip after refactoring". The comments highlight that this change leads to a misleading function signature for _get_sparkle_zip_download_url because the version and platform parameters are no longer used, recommending their removal for clarity and correctness.
| # Look for the Sparkle ZIP file: {version}-{platform}.zip | ||
| expected_filename = f"{version}-{platform}.zip" | ||
| # Look for the Sparkle ZIP file: Omi.zip | ||
| expected_filename = f"Omi.zip" |
There was a problem hiding this comment.
This change hardcodes the expected_filename to "Omi.zip". Consequently, the version and platform parameters passed to _get_sparkle_zip_download_url are no longer used in constructing this filename. This creates a misleading function signature, as it implies these parameters are still relevant for identifying the Sparkle ZIP file. This can lead to confusion and potential future correctness issues if the asset naming convention changes or if developers assume these parameters are actively used. Consider updating the function signature to remove these unused parameters if they are not needed for any other part of the Sparkle ZIP identification logic.
| for asset in assets: | ||
| asset_name = asset.get("name", "") | ||
| if asset_name.endswith(f"-{platform}.zip"): | ||
| if asset_name.endswith(f"Omi.zip"): |
There was a problem hiding this comment.
This fallback logic also hardcodes the asset name suffix to "Omi.zip", further confirming that the platform parameter is no longer used in determining the Sparkle ZIP file. This reinforces the concern about the misleading function signature of _get_sparkle_zip_download_url regarding the version and platform parameters.
No description provided.