Fix startup-css-check as-if-foss rules
- Feedback issue
- Full context MR (permanent draft)
Update startup css which fixes login pages-
Add generate:startup_css yarn script
Add startup-css-check in CI (depends on parent MR)
- Fix startup-css-check as-if-foss rules
- Add startup_css Dangerfile
What does this MR do?
This MR fixes the startup-css-check as-if-foss
CI rule conflict by setting it to the same rule as the rspec frontend_fixture as-if-foss
.
https://gitlab.com/gitlab-org/gitlab/-/pipelines/313703042
We also add a notice on the job failure message that the as-if-foss
build should be run if this job fails on EE.
We also revert the revert by cherry-picking the commits from !62836 (merged).
How do I know this purely reverts the revert?
You can diff the two diffs. This should verify that the only thing we're adding here is the new commit 818f397a.
(be on the lookout for -+
, +-
, ++
, and --
)
$ diff -u <(curl -s https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62836.diff) <(curl -s https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63069.diff)
--- /dev/fd/63 2021-06-02 11:02:29.000000000 -0500
+++ /dev/fd/62 2021-06-02 11:02:29.000000000 -0500
@@ -1,5 +1,5 @@
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml
-index 33aab8554e732c8428ba4574243eb24fe8b3f9ec..a772c1b847f1181f8cf7dcc2b7930c145c336b50 100644
+index 33aab8554e732c8428ba4574243eb24fe8b3f9ec..dfd595c26960ac38fae4c544cdfd2c6286f2fedc 100644
--- a/.gitlab/ci/frontend.gitlab-ci.yml
+++ b/.gitlab/ci/frontend.gitlab-ci.yml
@@ -317,3 +317,30 @@ bundle-size-review:
@@ -18,7 +18,7 @@
+startup-css-check:
+ extends:
+ - .startup-css-check-base
-+ - .frontend:rules:startup-css-check
++ - .frontend:rules:default-frontend-jobs
+ needs:
+ - job: "compile-test-assets"
+ - job: "rspec frontend_fixture"
@@ -29,32 +29,10 @@
+ extends:
+ - .startup-css-check-base
+ - .as-if-foss
-+ - .frontend:rules:startup-css-check-as-if-foss
++ - .frontend:rules:default-frontend-jobs-as-if-foss
+ needs:
+ - job: "compile-test-assets as-if-foss"
+ - job: "rspec frontend_fixture as-if-foss"
-diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml
-index fbc010f90b5fa03a5747fd9cb8647185d515db37..0a937f132cdc8fff407dce440e02d9dd48ffb241 100644
---- a/.gitlab/ci/rules.gitlab-ci.yml
-+++ b/.gitlab/ci/rules.gitlab-ci.yml
-@@ -485,6 +485,17 @@
- changes: *frontend-build-patterns
- allow_failure: true
-
-+.frontend:rules:startup-css-check:
-+ rules:
-+ - changes: *code-backstage-qa-patterns
-+
-+.frontend:rules:startup-css-check-as-if-foss:
-+ rules:
-+ - <<: *if-not-ee
-+ when: never
-+ - <<: *if-merge-request
-+ changes: *code-backstage-qa-patterns
-+
- ################
- # Memory rules #
- ################
diff --git a/.stylelintrc b/.stylelintrc
index 13ec6ea340b34b2a65309fd166b665e3883358ae..a4331811eb30e22169e23c45bba11b563a10fc7a 100644
--- a/.stylelintrc
@@ -11996,7 +11974,7 @@
+@import "startup/cloaking";
+@include cloak-startup-scss(none);
diff --git a/package.json b/package.json
-index 067a97450269fe3a8f1830fdde36f08d9e9ccb8d..e1cf76d6d2597b5c0a9adaa18d8ecea85c11481a 100644
+index 25ff42eae2eccf51ad76b08e5042baaf58851c38..ef4b3dcb4aba0741f1e60a685dc043c772528680 100644
--- a/package.json
+++ b/package.json
@@ -3,6 +3,7 @@
@@ -12105,10 +12083,10 @@
},
diff --git a/scripts/frontend/startup_css/startup_css_changed.sh b/scripts/frontend/startup_css/startup_css_changed.sh
new file mode 100755
-index 0000000000000000000000000000000000000000..b1df97f2f42c255d3e5a48d913663103ebdca8ef
+index 0000000000000000000000000000000000000000..d5f8ef3e9bcdb3b775e1ef8aeb7f1703de8e3ece
--- /dev/null
+++ b/scripts/frontend/startup_css/startup_css_changed.sh
-@@ -0,0 +1,36 @@
+@@ -0,0 +1,40 @@
+#!/bin/sh
+
+echo "-----------------------------------------------------------"
@@ -12132,7 +12110,11 @@
+It looks like there have been recent changes which require
+regenerating the Startup CSS files.
+
-+Consider one of the following options:
++**What should I do now?**
++
++IMPORTANT: Please make sure to update your MR title with `[RUN AS-IF-FOSS]` and start a new MR pipeline
++
++To fix this job, consider one of the following options:
+
+ 1. Regenerating locally with "yarn run generate:startup_css".
+ 2. Copy and apply the following diff:
I also took this for a smoke test on the review app and everything seems to look good
Context
From Slack:
remy:
Yeah, the startup-css-check as-if-foss rules should match the rules of rspec frontend_fixture as-if-foss (edited)
remy:
I think we can just use the .frontend:rules:default-frontend-jobs-as-if-foss rule for startup-css-check as-if-foss.
pslaughter:
I think we'd like to run the startup-css-check as-if-foss more often... If it fails for EE, likely the CE one will need to be updated to but we won't catch it unless it's explicitly [RUN AS-IF-FOSS]
pslaughter:
I think we can just use the .frontend:rules:default-frontend-jobs-as-if-foss rule for startup-css-check as-if-foss.
I'm down with trying this out, I just have some concerns about it :shrug:The alternative would be to run the fixtures more frequently (which I understand is not desirable):
remy:
Doesn’t yarn run generate:startup_css generate files for FOSS and EE?
pslaughter:
It requires CSS and HTML from the current environment's build, so it can technically only generate for one environment at a time :neutral_face:BUT! There's not even a different between EE and CE right now, but it's very possible that there could be. This split is an effort to protect ourselves from breaking FOSS build in the future. (edited)
Original MR description with screenshots
From !62836 (merged):
What does this MR do?
This MR adds the startup-css-check
CI job which verifies that the generated files in app/assets/stylesheets/startup/
are up-to-date.
Why are there so many file changes in this MR!?
It's been a long time since we've updated Startup CSS and it used to use a semi-manual process. This MR establishes the new baseline using the new Startup CSS generation.
Screenshots (strongly suggested)
How to test?
You can either:
- Simply open a page locally and check for any significant style changes while the
application.css
loads. - Try applying the following patch and visiting a page with
?startup_css_only=true
to prevent theapplication.css
from loading. You can then compare this page with one that does not have the?startup_css_only
parameter and compare for differences.
0001-Add-startup_css_only-condition.patch
You'll notice that updating the startup CSS in this MR fixes the following experiences on initial page load:
- Initial colors when dark mode
- Sidebar refactor
- Identicon on sidebar
- Register page
There's some things which we're choosing not to fix in this MR, namely the startup CSS for when a GitLab instance has a system header/footer and when the performance bar is open.
Does this MR meet the acceptance criteria?
Conformity
-
I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed.