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(cc-elasticsearch-info)!: rework properties to avoid impossible states #1052

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

Conversation

florian-sanders-cc
Copy link
Contributor

@florian-sanders-cc florian-sanders-cc commented May 7, 2024

What does this PR do?

  • Refactors the cc-elasticsearch-info component to implement our new state structure,
  • Implements TypeChecking within the cc-elasticsearch-info component and its stories.

How to review?

  • Check the commit (the diff may not be easy to read so you can review the final result locally if you prefer),
  • If you check it locally, you should not get any Typescript error within the component file or the story file,
  • Compare the prod stories to the preview stories.

@florian-sanders-cc florian-sanders-cc added this to the State migration milestone May 7, 2024
@florian-sanders-cc florian-sanders-cc self-assigned this May 7, 2024
Copy link
Contributor

github-actions bot commented May 7, 2024

🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/cc-elasticsearch-info/state-migration/index.html.

This preview will be deleted once this PR is closed.

Copy link
Member

@hsablonniere hsablonniere left a comment

Choose a reason for hiding this comment

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

Hey @florian-sanders-cc and @HeleneAmouzou, thank you for this 🎉

I added one comment, feel free to apply it or not.

Copy link
Member

@Galimede Galimede left a comment

Choose a reason for hiding this comment

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

Thanks for this, LGTM! 💪 🎉

…le states

BREAKING CHANGE: the properties have changed
- `state`: new property containing the whole state
- `error`: property has been deleted as it is now part of the state
- `links`: property has been deleted as it is now part of the state
@florian-sanders-cc florian-sanders-cc force-pushed the cc-elasticsearch-info/state-migration branch from bddf517 to c49c99c Compare June 3, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants