Skip to content
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

refactor: [M3-8640] – Move Theme layer from manager package to ui package #11092

Conversation

dwiley-akamai
Copy link
Contributor

@dwiley-akamai dwiley-akamai commented Oct 11, 2024

Description πŸ“

Move the Theme layer to the recently-created (#11057) linode/ui package.

Target release date πŸ—“οΈ

10/28/24

How to test πŸ§ͺ

Ensure that you have no issues building and running the app locally and that CM looks good visually. CI checks should also pass.

As an Author I have considered πŸ€”

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@dwiley-akamai dwiley-akamai marked this pull request as ready for review October 11, 2024 21:02
@dwiley-akamai dwiley-akamai requested review from a team as code owners October 11, 2024 21:02
@dwiley-akamai dwiley-akamai requested review from AzureLatte, carrillo-erik, coliu-akamai and jaalah-akamai and removed request for a team October 11, 2024 21:02
Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

βœ… Theme is looking good for light/dark
βœ… Imports from @linode/ui appear to be referencing exports properly

@bnussman-akamai
Copy link
Member

bnussman-akamai commented Oct 14, 2024

Hmm. The component test pipeline it failing

@dwiley-akamai
Copy link
Contributor Author

Hmm. The component test pipeline it failing

Yeah, based on the logs this is the issue:

Error: The following dependencies are imported but could not be resolved:

  @linode/ui (imported by /home/node/app/packages/manager/src/utilities/theme.ts)

Are they installed?

Looking into what's causing it but nothing firm yet

Copy link

github-actions bot commented Oct 14, 2024

Coverage Report: βœ…
Base Coverage: 86.96%
Current Coverage: 86.96%

Comment on lines -70 to -72
- ./packages/manager:/home/node/app/packages/manager
- ./packages/validation:/home/node/app/packages/validation
- ./packages/api-v4:/home/node/app/packages/api-v4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nice find. This didn't even occur to me

packages/manager/tsconfig.json Outdated Show resolved Hide resolved
@jaalah-akamai jaalah-akamai added the Approved Multiple approvals and ready to merge! label Oct 15, 2024
@dwiley-akamai dwiley-akamai merged commit f6f5e97 into linode:develop Oct 15, 2024
21 of 23 checks passed
@dwiley-akamai dwiley-akamai deleted the M3-8640-modularization-move-theme-to-ui-package branch October 15, 2024 22:00
Copy link

cypress bot commented Oct 15, 2024

Cloud Manager E2EΒ  Β  Run #6677

Run Properties:Β  status check failedΒ FailedΒ #6677 Β β€’Β  git commit f6f5e97bc2: refactor: [M3-8640] – Move Theme layer from `manager` package to `ui` package (#...
Project Cloud Manager E2E
Run status status check failedΒ FailedΒ #6677
Run duration 28m 35s
Commit git commit f6f5e97bc2: refactor: [M3-8640] – Move Theme layer from `manager` package to `ui` package (#...
Committer Dajahi Wiley
View all properties for this run β†—οΈŽ

Test results
Tests that failedΒ  Failures 2
Tests that were flakyΒ  Flaky 6
Tests that did not run due to a developer annotating a test with .skipΒ  Pending 2
Tests that did not run due to a failure in a mocha hookΒ  Skipped 0
Tests that passedΒ  Passing 433

Tests for review

FailedΒ  oneClickApps/one-click-apps.spec.ts β€’ 1 failed test

View Output Video

Test Artifacts
OneClick Apps (OCA) > Lists all the OneClick Apps Screenshots Video
FailedΒ  helpAndSupport/open-support-ticket.spec.ts β€’ 1 failed test

View Output Video

Test Artifacts
open support tickets > can create an SMTP support ticket Screenshots Video
FlakinessΒ  linodes/switch-linode-state.spec.ts β€’ 1 flaky test

View Output Video

Test Artifacts
switch linode state > reboots a linode from landing page Screenshots Video
FlakinessΒ  objectStorageGen2/bucket-create-gen2.spec.ts β€’ 3 flaky tests

View Output Video

Test Artifacts
Object Storage Gen2 create bucket tests > can create a bucket with E0 endpoint type Screenshots Video
Object Storage Gen2 create bucket tests > can create a bucket with E2 endpoint type Screenshots Video
Object Storage Gen2 create bucket tests > can create a bucket with E3 endpoint type Screenshots Video
FlakinessΒ  placementGroups/delete-placement-groups.spec.ts β€’ 1 flaky test

View Output Video

Test Artifacts
Placement Group deletion > can delete with Linodes assigned when unexpected error show up and retry Screenshots Video
FlakinessΒ  volumes/delete-volume.spec.ts β€’ 1 flaky test

View Output Video

Test Artifacts
volume delete flow > deletes a volume Screenshots Video

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Modularization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants