Skip to content

Fix startup-css-check as-if-foss rules

Paul Slaughter requested to merge ps-fix-startup-css-check-rules into master

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 😄

Screen_Shot_2021-06-02_at_11.45.45_AM

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:

  1. Simply open a page locally and check for any significant style changes while the application.css loads.
  2. Try applying the following patch and visiting a page with ?startup_css_only=true to prevent the application.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

Description Before (master with ?startup_css_only=true) After (this MR with ?startup_css_only=true) After (this MR with full load)
Sign in before_sign_in_startup_only after_sign_in_after_only after_sign_in_full
Register before_register_startup_only after_register_startup_only after_register_full
Project page + signed out + Feature.enable(:combined_menu) before_project_anon_combined_startup_only after_project_anon_combined_startup_only after_project_anon_combined_full
Project page before_project_startup_only after_project_startup_only after_project_full
Project page + sidebar collapse before_sidebar_collapse_startup_only after_sidebar_collapse_startup_only after_sidebar_collapse_full
Project page + Feature.enable(:sidebar_refactor) before_sidebar_refactor_startup_only after_sidebar_refactor_startup_only after_sidebar_refactor_full
Project page + Feature.enable(:combined_menu) before_combined_menu_startup_only after_combined_menu_startup_only after_combined_menu_full
Dark mode before_dark_startup_only after_dark_startup_only after_dark_full

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

Availability and Testing

Edited by Paul Slaughter

Merge request reports