-
Notifications
You must be signed in to change notification settings - Fork 16
Creation of desktop application for windows and mac #340
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
|
great job! can you add linux desktop? |
Sorry @grunch was afk! Sure I will add linux desktop asap |
WalkthroughAdds a GitHub Actions workflow that builds, packages, and releases Flutter desktop apps for Linux, macOS, and Windows on version tag pushes or manual dispatch; includes SDK setup, dependency installs, build_runner, optional analysis, platform-specific build/packaging, artifact upload, and release creation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Event as Tag Push / Manual
participant GHA as GitHub Actions
participant Repo as Repository
participant SDK as Flutter SDK
participant Tools as Pub / build_runner
participant Builder as Platform Build (Linux/macOS/Windows)
participant Pack as Packager (dist/)
participant Release as GitHub Release
Event->>GHA: trigger workflow (tag or dispatch)
GHA->>Repo: checkout code
GHA->>SDK: setup Flutter (stable, version)
GHA->>GHA: accept licenses & cache deps
GHA->>Tools: install deps, run build_runner
GHA->>GHA: extract version from pubspec.yaml
GHA->>GHA: optional code analysis (allowed to fail)
GHA->>Builder: perform platform-specific release build
Builder-->>GHA: build artifacts
GHA->>Pack: package artifacts into dist/ (tar.gz / zip)
Pack-->>GHA: packaged archives
GHA->>Release: upload artifacts & create/update release (tag = v{version})
Release-->>Event: release published
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/linux.yml (1)
24-28: Consider pinning Linux build dependency versions for reproducibility.The Linux dependencies are installed without version constraints using apt-get, which means builds may differ over time as packages update. While this ensures compatibility with latest patches, it can also introduce non-deterministic behavior. For CI/CD reliability, consider pinning dependency versions or explicitly documenting that latest versions are acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/MacOs.yml(1 hunks).github/workflows/linux.yml(1 hunks).github/workflows/windows.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/MacOs.yml (1)
38-50: Version extraction and artifact naming — verify cross-platform consistency with Windows implementation.The version extraction logic correctly uses
grepwith anchored pattern and removes carriage returns for cross-platform compatibility. Both$GITHUB_ENVand$GITHUB_OUTPUTare set properly for downstream step references. Ensure the Windows workflow's different extraction approach (PowerShell) produces identical version strings..github/workflows/windows.yml (2)
30-35: Verify or refactor the Android license acceptance command.The PowerShell syntax
"y" * 100creates a string of 100 consecutive 'y' characters, which may not function as intended for interactive license prompts that expect separate responses per agreement. Since the step hascontinue-on-error: true, build failures here won't block the workflow.Consider: (1) verifying this command actually works as intended; (2) removing it if Android SDK licenses aren't needed for Windows desktop builds; or (3) using a more explicit PowerShell approach to handle interactive prompts.
45-54: Verify Windows version extraction produces identical output to Unix implementations.The Windows PowerShell version extraction uses a different approach than the bash scripts in macOS and Linux workflows:
- Unix (MacOs.yml, linux.yml):
grep '^version: ' pubspec.yaml | cut -d ' ' -f 2 | tr -d '\r'- Windows (this file):
(Select-String -Path pubspec.yaml -Pattern '^version: ').Line -replace 'version: ','' | Trim()While both are platform-appropriate, they must produce identical version strings for consistent release tagging. Edge cases like trailing whitespace or line endings could cause mismatches. Verify the extracted versions are identical across platforms, especially if pubspec.yaml has non-standard formatting.
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/desktop.yml (1)
41-45: Consider making Flutter version configurable and add dependency caching.The hardcoded Flutter version (3.32.2) reduces flexibility for testing with newer versions. Additionally, there's no caching of Flutter or pub dependencies, which adds build time overhead on every run.
Apply this diff to add caching and make the version configurable:
- name: Setup Flutter uses: subosito/flutter-action@v2 with: channel: stable - flutter-version: '3.32.2' + flutter-version: '3.32.2' # Set to 'stable' or use a workflow input for flexibility + + - name: Cache Flutter dependencies + uses: actions/cache@v4 + with: + path: ~/.pub-cache + key: ${{ runner.os }}-pub-${{ hashFiles('**/pubspec.lock') }} + restore-keys: ${{ runner.os }}-pub-Optionally, consider using a workflow input for version:
on: push: tags: ['v*.*.*'] workflow_dispatch: inputs: flutter-version: description: 'Flutter version (default: 3.32.2)' required: false default: '3.32.2'Then in the Setup Flutter step:
flutter-version: ${{ github.event.inputs.flutter-version || '3.32.2' }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/desktop.yml(1 hunks)
🔇 Additional comments (2)
.github/workflows/desktop.yml (2)
121-136: Ensure artifact paths are consistent between creation and upload.The artifact upload step (line 126) searches for files at a relative path that depends on the current working directory. After the packaging steps create files with
cdand../../../../../navigation, the path reference in line 126 may not resolve correctly.This issue is resolved by the packaging refactor suggested in the previous comment. Verify that the dist directory path is used consistently in both packaging and upload steps.
1-137: Based on the verification conducted, I found a valid concern about workflow duplication. Matrix workflows eliminate separate jobs for each configuration by defining them all in one place, making workflow files cleaner and easier to read while simplifying maintenance. The script confirmed that separate platform-specific workflows (macos.yml, linux.yml, windows.yml) exist alongside the new multi-platform desktop.yml.However, without access to documentation about the repository's CI/CD consolidation strategy, trigger conditions for each workflow, or maintenance status of the older files, I cannot definitively determine whether this represents an active maintenance burden or a transitional deprecation.
Clarify the intended CI/CD workflow strategy and deprecation timeline for platform-specific workflows.
Before merging this multi-platform desktop.yml workflow, document whether the existing platform-specific workflows (macos.yml, linux.yml, windows.yml) should be:
- Deprecated and removed after this workflow is stable
- Maintained in parallel for different purposes (clarify why)
- Already replaced but not yet cleaned up
Having multiple workflows for the same purpose leads to maintenance challenges; a unified matrix approach simplifies modifications since updates only need to occur in one workflow configuration. If both approaches will coexist, ensure they remain synchronized and document the division of responsibilities.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/desktop.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/desktop.yml (1)
107-118: ✅ Version extraction validation addressed previous concern.The addition of the version check (lines 112-114) correctly prevents silent failures when version extraction fails. Good defensive programming.
grunch
left a comment
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.
LGTM
Hi @grunch , @AndreaDiazCorreia , @Catrya ,
this morning i spent sometime to improve gh actions to generate mobile desktop versions for:
the result is here in my fork of mobile, if you want to add this as a feature merge this pr, please consider that I am totally noob on MacOs so I am not aware of the best practise to generate for their MacOs.
Summary by CodeRabbit