-
Notifications
You must be signed in to change notification settings - Fork 1
add build matrix check #90
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
Added an entry to the .gitignore file to permit the inclusion of local development configuration files, enhancing the flexibility for developers working in local environments. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
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.
Pull Request Overview
This PR introduces a new SPECIAL_BUILD_APPS category for applications with custom build logic (currently containing notify_push), and adds a new JSON matrix generator for external apps configuration. The main changes improve the organization and flexibility of the build system.
Key changes:
- Introduces
SPECIAL_BUILD_APPScategory for apps requiring custom build targets, improving separation of concerns from standard build categories - Adds
generate_external_apps_matrix_jsontarget to complement the existing YAML matrix generator - Updates validation and build targets to include the new special apps category
Reviewed Changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Makefile | Adds SPECIAL_BUILD_APPS category, integrates it throughout build/validation logic, updates matrix generation to sort apps and include special apps, adds new JSON matrix generator, and redirects informational messages to stderr |
| .gitignore | Adds exception for local developer configuration file at /configs/localdev.config.php |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This commit introduces a new target to generate an external apps matrix in JSON format, enhancing the build process by providing a structured output of app configuration details. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…stderr Ensure that the message indicating the generation of the external apps matrix YAML file is sent to standard error for better visibility during execution. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
This change modifies the Makefile to sort the list of configured apps alphabetically before iterating through them. This ensures a consistent order when processing the apps. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
- Introduced SPECIAL_BUILD_APPS variable to define apps with custom build logic. - Updated build targets to include special build targets in the build_all_external_apps target. - Adjusted app validation and configuration checks to account for special build apps. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Added a .precheck target to ensure required directories and files exist before executing build tasks. This improves error handling and user guidance when the Makefile is run from an incorrect directory. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
381c1ff to
6358493
Compare
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.
Pull Request Overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ "$$found_any" = true ]; then \ | ||
| echo " }"; \ | ||
| fi; \ | ||
| echo "]"' | jq 'sort_by(.name)' |
Copilot
AI
Nov 11, 2025
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.
The JSON generation relies on jq being available in the environment, but there's no error handling if jq is missing. The .precheck target validates Nextcloud directories but doesn't check for required tools like jq. Consider adding a check for jq availability in .precheck or adding error handling to this target to provide a helpful message if jq is not installed.
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.
@copilot open a new pull request to apply changes based on this feedback
|
@printminion-co I've opened a new pull request, #91, to work on those changes. Once the pull request is ready, I'll request review from you. |
No description provided.