-
Notifications
You must be signed in to change notification settings - Fork 1
fix app config password policy #89
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
496f705 to
148b2e4
Compare
Co-authored-by: printminion-co <[email protected]>
- Introduced a new target for apps that require composer installation without running scripts. - Updated build targets to include the new no-scripts category. - Enhanced output messages to reflect the new build process for composer-only apps. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Removed 'forms' and 'twofactor_totp' from the app lists, and added 'forms' to the COMPOSER_NO_SCRIPTS_APPS for improved build handling. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
- Introduced a new target `validate_app_list_uniqueness` to ensure apps are only listed in one category without duplication. - Updated help text for validation targets in the Makefile. - Integrated the new validation into the existing `validate_all` target. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
148b2e4 to
300b916
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
This PR fixes the app configuration for apps with password policy issues by introducing a new COMPOSER_NO_SCRIPTS_APPS category. The change addresses build failures where apps use @composer bin commands in their post-install scripts but have the bamarni/composer-bin-plugin only in require-dev, causing failures when running composer install --no-dev.
Key Changes:
- Introduced
COMPOSER_NO_SCRIPTS_APPScategory for apps requiring the--no-scriptsflag during composer installation - Moved
forms,password_policy, andtwofactor_totpfromFULL_BUILD_APPSto the new category - Refactored validation logic from inline bash in Makefile to standalone scripts for maintainability
- Added uniqueness validation to prevent apps from being in multiple lists
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/validate_external_apps.sh | New standalone script that validates external app configuration and suggests proper categorization |
| scripts/validate_app_list_uniqueness.sh | New standalone script that validates apps appear in only one list and checks for hardcoded target conflicts |
| Makefile | Added COMPOSER_NO_SCRIPTS_APPS category, moved problematic apps, refactored validation to use external scripts, and added jq precheck |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
seriAlizations
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
… logic into standalone script. - Added `validate_external_apps.sh` to validate external apps and suggest proper categorization based on their configuration. - Updated Makefile to call the new validation scripts for improved app management. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
6cd1c45 to
09f52c3
Compare
No description provided.