PS-906 add zip assets on release #642
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to generate and distribute documentation zip assets on release. The changes enable documentation generation from a Symfony command with file output capability and integrate this into the CI workflow to create zip assets for both push events and releases.
- Enhanced
DocumentationDumperCommandto support output filename option for file-based documentation generation - Modified CI workflow to trigger on releases and generate documentation zip assets
- Added conditional execution logic to separate push and release workflows
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| databox/api/src/Command/DocumentationDumperCommand.php | Added file output option and directory creation logic for documentation generation |
| databox/api/.gitignore | Excluded generated documentation directory from version control |
| .github/workflows/ci.yaml | Added release triggers, documentation generation steps, and conditional workflow execution |
| repo_token: ${{ secrets.GITHUB_TOKEN }} | ||
| file: ${{ github.workspace }}/result.zip | ||
| asset_name: phrasea-doc.zip | ||
| tag: ${{ github.ref }} |
There was a problem hiding this comment.
The tag reference ${{ github.ref }} will include the full ref path (e.g., 'refs/tags/v1.0.0') but the upload-release-action expects just the tag name. Use ${{ github.ref_name }} instead to get only the tag name.
| tag: ${{ github.ref }} | |
| tag: ${{ github.ref_name }} |
Copilot uses AI. Check for mistakes.
|
|
||
| $outputFilename = $input->getOption('output-filename'); | ||
|
|
||
| $outputDir = __DIR__.'/../../_generatedDoc'; |
There was a problem hiding this comment.
The hardcoded relative path __DIR__.'/../../_generatedDoc' is fragile and could break if the file structure changes. Consider using a configurable parameter or a more robust path resolution method.
| $outputDir = __DIR__.'/../../_generatedDoc'; | |
| $outputDir = $input->getOption('output-dir') ?: realpath(__DIR__ . '/../../_generatedDoc'); | |
| if ($outputDir === false) { | |
| $outputDir = __DIR__ . '/../../_generatedDoc'; | |
| } |
Copilot uses AI. Check for mistakes.
| $outputDir = __DIR__.'/../../_generatedDoc'; | ||
|
|
||
| if (!is_dir($outputDir)) { | ||
| if (!mkdir($outputDir, 0755, true) && !is_dir($outputDir)) { |
There was a problem hiding this comment.
The directory creation logic has a race condition. Between the !is_dir($outputDir) check on line 40 and the mkdir() call, another process could create the directory, causing mkdir() to fail but is_dir() to return true. Consider using @mkdir($outputDir, 0755, true) and only check for errors if the directory still doesn't exist.
Copilot uses AI. Check for mistakes.
| @@ -255,11 +260,11 @@ jobs: | |||
| bin/setup.sh test | |||
|
|
|||
| - name: Test | |||
| if: "!contains(github.event.head_commit.message, '[skip test]') && !contains(github.event.head_commit.message, '[skip php-test]')" | |||
| if: "!contains(github.event.head_commit.message, '[skip test]') && !contains(github.event.head_commit.message, '[skip php-test]') && github.event_name == 'push'" | |||
|
|
||
| $outputFilename = $input->getOption('output-filename'); | ||
|
|
||
| $outputDir = __DIR__.'/../../_generatedDoc'; |
| $outputDir = __DIR__.'/../../_generatedDoc'; | ||
|
|
||
| if (!is_dir($outputDir)) { | ||
| if (!mkdir($outputDir, 0755, true) && !is_dir($outputDir)) { |
No description provided.