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

feat: introduce new package @commercetools-backend/express with utilities for working with Custom Applications #1447

Merged
merged 6 commits into from
Apr 20, 2020

Conversation

emmenko
Copy link
Member

@emmenko emmenko commented Apr 17, 2020

Ref #1444

cc @adnasa for awareness

@vercel
Copy link

vercel bot commented Apr 17, 2020

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

🔍 Inspect: https://zeit.co/commercetools/merchant-center-application-kit/5b4l43ysh
✅ Preview: https://merchant-center-application-kit-git-nm-server-express.commercetools.now.sh

@emmenko emmenko added the 🚀 Type: New Feature Something new label Apr 17, 2020
@vercel vercel bot temporarily deployed to Preview April 17, 2020 21:13 Inactive
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.

Good stuff! This will help builders a lot :)

packages/server-express/README.md Outdated Show resolved Hide resolved
packages/server-express/README.md Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview April 20, 2020 10:11 Inactive
Copy link
Member Author

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

Alright, I addressed your feedback and moved the package to packages-backend

@@ -0,0 +1,45 @@
# @commercetools-backend/server-express
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, do we do express or server-express? Intuitively I would just go with express 🤷 😕

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I guess with the "backend" prefix we can just use express

}
const cloudIdentifierHeader = request.header('x-mc-api-cloud-identifier');
const issuer =
options.inferIssuer && cloudIdentifierHeader
Copy link
Contributor

Choose a reason for hiding this comment

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

Danke 🖤!

default:
return undefined;
}
};
const throwIfIssuerIsNotAValidUrl = (issuer: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🏨

@vercel vercel bot temporarily deployed to Preview April 20, 2020 13:05 Inactive
@emmenko
Copy link
Member Author

emmenko commented Apr 20, 2020

So for further work on the package, I suppose we can merge this and open other PRs?

@emmenko emmenko requested a review from tdeekens April 20, 2020 13:06
@emmenko emmenko marked this pull request as ready for review April 20, 2020 13:07
@emmenko emmenko changed the title feat: introduce new package server-express with utilities for working with Custom Applications feat: introduce new package @commercetools-backend/express with utilities for working with Custom Applications Apr 20, 2020
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.

Yes, I think this is great already for a start! Thanks!

@vercel vercel bot temporarily deployed to Preview April 20, 2020 15:09 Inactive
@vercel vercel bot temporarily deployed to Preview April 20, 2020 15:30 Inactive
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.

Was commiting the build on propose or are we missing a .gitignore entry? 😄

@emmenko emmenko merged commit 2bf72e8 into master Apr 20, 2020
@emmenko emmenko deleted the nm-server-express branch April 20, 2020 17:55
@emmenko
Copy link
Member Author

emmenko commented Apr 20, 2020

Just a mistake 😅

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

Successfully merging this pull request may close these issues.

2 participants