539 improve gitlab ci #414
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR improves GitLab CI code-quality and dependency scanning integration (including new stylelint, Twig-CS-Fixer, and GitLab-native report formats), adds the necessary tooling dependencies and configs, and cleans up SCSS/Twig code style (with some minor layout tweaks) to satisfy the new linters. Flow diagram for updated code-quality and scanning jobsflowchart TD
Start[Start pipeline] --> Build[Install dependencies and build assets]
subgraph Code_quality
direction TB
CQ_phpcs[PHP_CodeSniffer
gitlab report format
phpcs gitlab]
CQ_phpstan[PHPStan
gitlab error format]
CQ_twigcs[Twig CS Fixer
junit report]
CQ_stylelint[Stylelint
GitLab codequality formatter]
CQ_standardjs[StandardJS
standard gitlab wrapper]
end
subgraph Dependency_scanning
direction TB
DS_npm[NPM audit]
DS_composer[Composer audit]
end
subgraph Outdated_packages
direction TB
OP_npm[NPM outdated]
OP_composer[Composer outdated]
end
subgraph Test
direction TB
T_phpunit[PHPUnit tests]
end
Build --> CQ_phpcs
Build --> CQ_phpstan
Build --> CQ_twigcs
Build --> CQ_stylelint
Build --> CQ_standardjs
Build --> DS_npm
Build --> DS_composer
Build --> OP_npm
Build --> OP_composer
Build --> T_phpunit
%% Failure visibility re runs
CQ_phpstan -->|on failure only rerun without gitlab format| CQ_phpstan_rerun[PHPStan rerun plain output]
CQ_twigcs -->|on failure only rerun with color output| CQ_twigcs_rerun[Twig CS Fixer rerun]
CQ_stylelint -->|on failure only rerun with color output| CQ_stylelint_rerun[Stylelint rerun]
End[Pipeline end]
T_phpunit --> End
OP_composer --> End
OP_npm --> End
DS_composer --> End
DS_npm --> End
CQ_standardjs --> End
CQ_stylelint_rerun --> End
CQ_twigcs_rerun --> End
CQ_phpstan_rerun --> End
CQ_phpcs --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The changes from
rgba(...)torgb(...)with four arguments in_bootstrap-variables.scss(e.g.$navbar-light-color,$toast-background-color, form validation focus box-shadow, etc.) produce invalid CSS; these should remainrgba(...)when an alpha channel is used. - The PHP_CodeSniffer and PHPStan jobs now generate GitLab-style JSON reports but are still declared under
artifacts: reports: junit:; to have GitLab interpret them correctly, switch these to the appropriate report type (e.g.codequality:) or revert to JUnit-compatible formats.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The changes from `rgba(...)` to `rgb(...)` with four arguments in `_bootstrap-variables.scss` (e.g. `$navbar-light-color`, `$toast-background-color`, form validation focus box-shadow, etc.) produce invalid CSS; these should remain `rgba(...)` when an alpha channel is used.
- The PHP_CodeSniffer and PHPStan jobs now generate GitLab-style JSON reports but are still declared under `artifacts: reports: junit:`; to have GitLab interpret them correctly, switch these to the appropriate report type (e.g. `codequality:`) or revert to JUnit-compatible formats.
## Individual Comments
### Comment 1
<location> `src/Frontend/Themes/Bootstrap/src/Layout/Sass/_bootstrap-variables.scss:922-931` </location>
<code_context>
+$navbar-light-color: rgb(var(--#{$prefix}emphasis-color-rgb), .65);
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `rgb()` instead of `rgba()` with `*-rgb` CSS variables and an alpha parameter will generate invalid CSS.
Several variables that used `rgba(var(--…-rgb), alpha)` were changed to `rgb(var(--…-rgb), alpha)` (e.g. `$navbar-light-color`, `$navbar-light-hover-color`, `$navbar-light-active-color`, `$navbar-light-disabled-color`, `$dropdown-dark-link-hover-bg`, `$card-cap-bg`, `$toast-background-color`, `$toast-header-background-color`). This `rgb()` form is invalid and will likely be ignored by browsers. To preserve the semi‑transparent effect and browser compatibility, keep these as `rgba(var(--#{$prefix}…-rgb), <alpha>)` or adopt the `rgb(r g b / a)` syntax consistently instead.
</issue_to_address>
### Comment 2
<location> `src/Frontend/Themes/Bootstrap/src/Layout/Sass/_bootstrap-variables.scss:1107-1116` </location>
<code_context>
+$form-invalid-border-color: $form-feedback-invalid-color;
$form-validation-states: (
- "valid": (
- "color": var(--#{$prefix}form-valid-color),
- "icon": $form-feedback-icon-valid,
- "tooltip-color": #fff,
- "tooltip-bg-color": var(--#{$prefix}success),
- "focus-box-shadow": 0 0 $input-btn-focus-blur $input-focus-width rgba(var(--#{$prefix}success-rgb), $input-btn-focus-color-opacity),
- "border-color": var(--#{$prefix}form-valid-border-color),
- ),
- "invalid": (
- "color": var(--#{$prefix}form-invalid-color),
- "icon": $form-feedback-icon-invalid,
- "tooltip-color": #fff,
- "tooltip-bg-color": var(--#{$prefix}danger),
- "focus-box-shadow": 0 0 $input-btn-focus-blur $input-focus-width rgba(var(--#{$prefix}danger-rgb), $input-btn-focus-color-opacity),
- "border-color": var(--#{$prefix}form-invalid-border-color),
- )
+ "valid": (
</code_context>
<issue_to_address>
**issue (bug_risk):** The `focus-box-shadow` entries in `form-validation-states` also use `rgb()` with an alpha argument and should stay `rgba()`.
Both the `"valid"` and `"invalid"` entries currently use:
```scss
"focus-box-shadow": 0 0 $input-btn-focus-blur $input-focus-width rgb(var(--#{$prefix}success-rgb), $input-btn-focus-color-opacity),
```
(and the danger equivalent). `rgb()` with four arguments is invalid, so the focus box-shadow color won’t render. Please switch these back to `rgba(var(--#{$prefix}…-rgb), $input-btn-focus-color-opacity)` (or the `rgb(r g b / a)` form) to preserve the intended focus style.
</issue_to_address>
### Comment 3
<location> `.gitlab-ci.yml:67-70` </location>
<code_context>
- - *before_script_test_configuration
script:
- - php bin/phpcs --report-full --report-junit=phpcs-report.xml
+ - php bin/phpcs --colors --report-full --report-\\Micheh\\PhpCodeSniffer\\Report\\Gitlab=phpcs-report.json
artifacts:
expire_in: 1 week
reports:
- junit: phpcs-report.xml
+ junit: phpcs-report.json
</code_context>
<issue_to_address>
**issue (bug_risk):** The PHPCS report configuration mixes a GitLab JSON formatter with a JUnit artifact type and the report CLI option looks unusual.
This job generates a GitLab PHPCS JSON report (`phpcs-report.json`) but publishes it as a `junit` artifact, which must be XML, so GitLab is unlikely to parse it correctly.
Consider:
1. If this is for GitLab Code Quality, publish `phpcs-report.json` under `codequality:` instead of `junit:`.
2. Adjust the PHPCS CLI to the more standard form: `--report=\Micheh\PhpCodeSniffer\Report\Gitlab --report-file=phpcs-report.json` rather than embedding the filename in the `--report` option.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+922
to
+931
| $navbar-light-color: rgb(var(--#{$prefix}emphasis-color-rgb), .65); | ||
| $navbar-light-hover-color: rgb(var(--#{$prefix}emphasis-color-rgb), .8); | ||
| $navbar-light-active-color: rgb(var(--#{$prefix}emphasis-color-rgb), 1); | ||
| $navbar-light-disabled-color: rgb(var(--#{$prefix}emphasis-color-rgb), .3); | ||
| $navbar-light-icon-color: rgba($body-color, .75); | ||
| $navbar-light-toggler-icon-bg: url("data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 30 30'><path stroke='#{$navbar-light-icon-color}' stroke-linecap='round' stroke-miterlimit='10' stroke-width='2' d='M4 7h22M4 15h22M4 23h22'/></svg>"); | ||
| $navbar-light-toggler-border-color: rgb(var(--#{$prefix}emphasis-color-rgb), .15); | ||
| $navbar-light-brand-color: $navbar-light-active-color; | ||
| $navbar-light-brand-hover-color: $navbar-light-active-color; | ||
| $navbar-dark-color: rgba($white, .55); |
There was a problem hiding this comment.
issue (bug_risk): Using rgb() instead of rgba() with *-rgb CSS variables and an alpha parameter will generate invalid CSS.
Several variables that used rgba(var(--…-rgb), alpha) were changed to rgb(var(--…-rgb), alpha) (e.g. $navbar-light-color, $navbar-light-hover-color, $navbar-light-active-color, $navbar-light-disabled-color, $dropdown-dark-link-hover-bg, $card-cap-bg, $toast-background-color, $toast-header-background-color). This rgb() form is invalid and will likely be ignored by browsers. To preserve the semi‑transparent effect and browser compatibility, keep these as rgba(var(--#{$prefix}…-rgb), <alpha>) or adopt the rgb(r g b / a) syntax consistently instead.
Comment on lines
1107
to
+1116
| // Deprecated in Bootstrap 5.2.0 for CSS variables | ||
| $popover-arrow-color: $popover-bg; | ||
| $popover-arrow-outer-color: var(--#{$prefix}border-color-translucent); | ||
| // fusv-enable | ||
|
|
||
| $popover-arrow-color: $popover-bg; | ||
| $popover-arrow-outer-color: var(--#{$prefix}border-color-translucent); | ||
|
|
||
| // Toasts | ||
|
|
||
| // scss-docs-start toast-variables | ||
| $toast-max-width: 350px; | ||
| $toast-padding-x: .75rem; | ||
| $toast-padding-y: .5rem; | ||
| $toast-font-size: .875rem; | ||
| $toast-color: null; | ||
| $toast-background-color: rgba(var(--#{$prefix}body-bg-rgb), .85); | ||
| $toast-border-width: var(--#{$prefix}border-width); | ||
| $toast-border-color: var(--#{$prefix}border-color-translucent); | ||
| $toast-border-radius: var(--#{$prefix}border-radius); | ||
| $toast-box-shadow: var(--#{$prefix}box-shadow); | ||
| $toast-spacing: $container-padding-x; | ||
|
|
||
| $toast-header-color: var(--#{$prefix}secondary-color); | ||
| $toast-header-background-color: rgba(var(--#{$prefix}body-bg-rgb), .85); | ||
| $toast-header-border-color: $toast-border-color; | ||
| // scss-docs-end toast-variables | ||
|
|
||
| $toast-max-width: 350px; | ||
| $toast-padding-x: .75rem; | ||
| $toast-padding-y: .5rem; | ||
| $toast-font-size: .875rem; | ||
| $toast-color: null; |
There was a problem hiding this comment.
issue (bug_risk): The focus-box-shadow entries in form-validation-states also use rgb() with an alpha argument and should stay rgba().
Both the "valid" and "invalid" entries currently use:
"focus-box-shadow": 0 0 $input-btn-focus-blur $input-focus-width rgb(var(--#{$prefix}success-rgb), $input-btn-focus-color-opacity),(and the danger equivalent). rgb() with four arguments is invalid, so the focus box-shadow color won’t render. Please switch these back to rgba(var(--#{$prefix}…-rgb), $input-btn-focus-color-opacity) (or the rgb(r g b / a) form) to preserve the intended focus style.
Comment on lines
+67
to
70
| - php bin/phpcs --colors --report-full --report-\\Micheh\\PhpCodeSniffer\\Report\\Gitlab=phpcs-report.json | ||
| artifacts: | ||
| expire_in: 1 week | ||
| reports: |
There was a problem hiding this comment.
issue (bug_risk): The PHPCS report configuration mixes a GitLab JSON formatter with a JUnit artifact type and the report CLI option looks unusual.
This job generates a GitLab PHPCS JSON report (phpcs-report.json) but publishes it as a junit artifact, which must be XML, so GitLab is unlikely to parse it correctly.
Consider:
- If this is for GitLab Code Quality, publish
phpcs-report.jsonundercodequality:instead ofjunit:. - Adjust the PHPCS CLI to the more standard form:
--report=\Micheh\PhpCodeSniffer\Report\Gitlab --report-file=phpcs-report.jsonrather than embedding the filename in the--reportoption.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://next-app.activecollab.com/108877/projects/62/tasks/241559
Summary by Sourcery
Update GitLab CI configuration and code quality tooling while applying style and linting fixes across SCSS and Twig templates.
New Features:
Enhancements:
Build:
CI: