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

docs: asset naming convention #178

Merged
merged 6 commits into from
Jan 7, 2025

Conversation

setchy
Copy link
Contributor

@setchy setchy commented Jan 5, 2025

closes #92

@setchy setchy requested a review from a team as a code owner January 5, 2025 07:02
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 98 to 114
### Windows Assets
- Asset must be a `.zip` file.
- Asset name must include `-win32` followed by the architecture:
- `-ia32` for 32-bit Windows.
- `-x64` for 64-bit Windows.
- `-arm64` for ARM-based Windows.

**Example asset names:**
- `app-win32-ia32.zip`
- `app-win32-x64.zip`
- `app-win32-arm64.zip`

#### Special Cases
- `.exe` files without specific architecture tags are assumed to be `-x64` by default.

**Example asset names:**
- `app-installer.exe` (treated as `-x64`).
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: same here with the default arch, I feel like we could move the contents of Special cases into Windows Assets and make it more consistent with macOS Assets.

Also a note: currently, the {{appName}}-win32-{{arch}}.*.exe pattern seems to be correctly recognized for all architectures, so in the code below I updated the extension requirements and included more examples.

### Windows Assets
- Asset must be a `.zip` or `.exe` file.
- Asset name must include the `-win32` platform identifier.
- Asset name may specify the architecture (if not specified, will default to `-x64`):
  - `-ia32` for 32-bit Windows.
  - `-x64` for 64-bit Windows.
  - `-arm64` for ARM-based Windows.
- `.exe` files without specific architecture tags or the `-win32` platform identifier are assumed to be `-x64` by default.

**Example asset names:**
- `app-win32-ia32.zip`
- `app-win32-x64.zip`
- `app-win32-arm64.zip`
- `app-win32-arm64.exe`
- `app-win32-arm64-v1.2.3.exe`
- `app-win32.exe` (no architecture specified - treated as `-x64`)
- `app-installer.exe` (generic `.exe` file with no architecture or platform identifier specified - treated as `-x64`)

We might want to update the error message for PLATFORM.WIN32 to something like No updates found (needs asset containing .*-win32-(x64|ia32|arm64).*.(zip|exe) or *.exe in public repository) in a follow-up PR.

Not sure about this one so I'll ask others in @electron/wg-ecosystem, but we might also want to lift the requirement for the -win32 platform specifier for .exe files - I assume this requirement is in place as a way to avoid confusion between .zip files for Windows and macOS, but that's not an issue with .exe files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

incorporated the suggested changes to the readme.

README.md Outdated Show resolved Hide resolved
Signed-off-by: Adam Setch <[email protected]>
@setchy setchy requested a review from erikian January 5, 2025 17:24
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
setchy and others added 4 commits January 6, 2025 09:50
Co-authored-by: Erik Moura <[email protected]>
Co-authored-by: Erik Moura <[email protected]>
Co-authored-by: Erik Moura <[email protected]>
Co-authored-by: Erik Moura <[email protected]>
@setchy setchy requested a review from erikian January 6, 2025 14:51
Copy link
Member

@erikian erikian left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@erikian erikian merged commit 9add95e into electron:main Jan 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation specifying asset naming conventions
2 participants