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: upgrade packages 3a - apps with breaking changes #571

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

michael-odonovan
Copy link
Contributor

@michael-odonovan michael-odonovan commented May 26, 2023

PR description

What is it doing?

upgrade packages with breaking changes ie. 1.4.2 => 2.1.0

Why is this required?

BAU

link to Jira ticket:

https://comicrelief.atlassian.net/browse/ENG-2543

Quick Checklist:

  • My PR title follows the Conventional Commit spec.

  • I have filled out the PR description as per the template above.

  • I have added tests to cover new or changed behaviour.

  • I have updated any relevant documentation.

Important! - lastly, make sure to squash merge...

@michael-odonovan michael-odonovan self-assigned this May 26, 2023
@michael-odonovan
Copy link
Contributor Author

michael-odonovan commented Jun 5, 2023

Some notes:

  • have used /* eslint-disable camelcase */ at the top of _MarketingPreferencesDS.js, as after upgrading eslint it throws a lot of errors around the mp_permissionEmail style naming because it wants camelCase naming. I have tried digging them out, however this creates a web of errors, so hopefully disabling one eslint rule for this file is a pragmatic solution? The mp_ is actually pretty handy here for clarity.

@michael-odonovan michael-odonovan force-pushed the upgrade_packages_3_a branch 2 times, most recently from 9ff1ea3 to 9f56231 Compare June 5, 2023 15:18
@michael-odonovan michael-odonovan force-pushed the upgrade_packages_3_a branch 3 times, most recently from aff4ce6 to 0611510 Compare June 8, 2023 15:49
@michael-odonovan
Copy link
Contributor Author

michael-odonovan commented Sep 6, 2023

Putting a maker in the sand this commit, with notes on the state of play:
d1fe44f

The good:

  • React, Styleguidist and Jest have been updated.

The bad:

  • fails jest tests, picture of error below...

The ugly:

  • Have spent a long time trying to sort this issue.
  • Spent an afternoon looking at it with Paul.
  • Have posted this error on Styleguidist repo, without reply from maintainer.

Screenshot 2023-09-06 at 13 58 35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant