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.

Sign up for GitHub

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

Jump to bottom

refactor: use compiled-data from uikit, expose compiled-data from appkit, use new data table #1685

Merged
merged 5 commits into from
Aug 19, 2020
Merged

refactor: use compiled-data from uikit, expose compiled-data from appkit, use new data table #1685

merged 5 commits into from
Aug 19, 2020

Conversation

emmenko
Copy link
Member

@emmenko emmenko commented Aug 18, 2020

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Aug 18, 2020 •
edited
Loading

🦋 Changeset is good to go

Latest commit: 709ea03

We got this.

This PR includes changesets to release 11 packages
Name Type
merchant-center-application-template-starter Minor
@commercetools-frontend/application-components Minor
@commercetools-frontend/application-shell Minor
@commercetools-frontend/i18n Minor
@commercetools-frontend/l10n Minor
@commercetools-frontend/mc-scripts Minor
@commercetools-frontend/react-notifications Minor
playground Minor
@commercetools-local/visual-testing-app Minor
@commercetools-website/custom-applications Minor
@commercetools-website/components-playground Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Aug 18, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/commercetools/merchant-center-application-kit/jhw8n62f3
✅ Preview: https://merchant-center-application-kit-git-nm-intl-compile-data.commercetools.vercel.app

tdeekens
tdeekens reviewed Aug 18, 2020
View reviewed changes
Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

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

Looks nice!

@vercel vercel bot temporarily deployed to Preview August 19, 2020 08:57 Inactive
emmenko
emmenko commented Aug 19, 2020
View reviewed changes
packages/react-notifications/src/components/notification-kinds/api-error-message/messages.ts
@@ -21,7 +21,7 @@ export default defineMessages({
description:
'User does a bulk update but someone else has saved changes for that element',
defaultMessage:
'Sorry, but we were unable to save your changes as someone else made changes to this same resource while you were editing.',
'Sorry, but we were unable to save your changes as someone else made changes to this same source while you were editing.',
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes simply reflect the values in the core.json, so that by running the extract command we don't overwrite/change the values.

@emmenko emmenko marked this pull request as ready for review August 19, 2020 10:07
@emmenko
Copy link
Member Author

emmenko commented Aug 19, 2020

Alright, I think this is good to go now. It might be that the mc apps need to be updated to latest uikit + appkit + react-intl versions, to have everything working fine.

tdeekens
tdeekens approved these changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

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

🥳🤩😎

mohib0306
mohib0306 approved these changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@mohib0306 mohib0306 left a comment

Choose a reason for hiding this comment

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

These changes looks good.

@mohib0306
Copy link
Contributor

mohib0306 commented Aug 19, 2020 •
edited
Loading

Just one thing about exporting data. If you remember the discussion about exporting countries lists (at least the english countries data) from our l10n package so that we could have better test data generation for MC-FE. Would it possible that that change is also added here

@emmenko
Copy link
Member Author

emmenko commented Aug 19, 2020

Yep, we can add that as well

@vercel vercel bot temporarily deployed to Preview August 19, 2020 12:56 Inactive
@vercel vercel bot temporarily deployed to Preview August 19, 2020 12:57 Inactive
@emmenko emmenko merged commit 4216b92 into master Aug 19, 2020
@emmenko emmenko deleted the nm-intl-compile-data branch August 19, 2020 19:57
@ghost ghost mentioned this pull request Aug 19, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdeekens tdeekens tdeekens approved these changes

@jonnybel jonnybel jonnybel approved these changes

@mohib0306 mohib0306 mohib0306 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emmenko @mohib0306 @tdeekens @jonnybel