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

39 Moving the Front end application from Go to Next.js #236

Merged
merged 30 commits into from Aug 9, 2022

Conversation

xoscar
Copy link
Contributor

@xoscar xoscar commented Jul 26, 2022

Fixes #39.

Changes

In this PR we are introducing the new front-end version. In this version, we are focusing on having a browser-side client written in Typescript.

The frontend was built based on the proposal presented during the last SIG call (July 25th 2022) and can be found here: https://docs.google.com/presentation/d/1klWGRAcFYJLBrXtMMRGHJvwQaMt1JPWnO7qQzFdJti4/edit?usp=sharing

Next Steps:

  1. Add instrumentation.
  2. Add unit tests and integration tests.

For significant contributions please make sure you have completed the following items:

Video demo

https://www.loom.com/share/c7b1c559346e4a4c8df4e670ef14b558

Screenshots

Screen Shot 2022-07-27 at 10 41 02 a m
Screen Shot 2022-07-27 at 10 42 02 a m

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 26, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@cartersocha
Copy link
Contributor

Make sure to go ahead & sign the cla @xoscar

.env Show resolved Hide resolved
@xoscar xoscar marked this pull request as ready for review July 27, 2022 16:02
@xoscar xoscar requested a review from a team as a code owner July 27, 2022 16:02
@reyang
Copy link
Member

reyang commented Jul 27, 2022

Thanks for your contribution, @xoscar!
Would you update the PR title to capture the intention? It's hard to understand what "39 web application" is unless someone reads through the details (and later it will cause pain if someone want to search old PRs).

@xoscar xoscar changed the title 39 web application 39 Moving the Front end application from Go to Next.js Jul 27, 2022
docker-compose.yml Outdated Show resolved Hide resolved
src/frontend/.env.local Outdated Show resolved Hide resolved
src/frontend/Dockerfile Outdated Show resolved Hide resolved
src/frontend/README.md Show resolved Hide resolved
src/frontend/gateways/Session.gateway.ts Show resolved Hide resolved
src/frontend/gateways/rpc/Ad.gateway.ts Show resolved Hide resolved
src/frontend/next.config.js Show resolved Hide resolved
src/frontend/protos/demo.ts Outdated Show resolved Hide resolved
src/frontend/utils/Request.ts Show resolved Hide resolved
@austinlparker
Copy link
Member

Could we get the checks passing on this to move it forward?

@julianocosta89
Copy link
Member

I believe that we can also get rid of the social network icons, we are not using them within the FE.

src/frontend/README.md Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/Dockerfile Outdated Show resolved Hide resolved
src/frontend/components/ProductPrice/ProductPrice.tsx Outdated Show resolved Hide resolved
src/frontend/pages/api/checkout.ts Show resolved Hide resolved
src/frontend/gateways/rpc/Cart.gateway.ts Show resolved Hide resolved
src/frontend/next-env.d.ts Outdated Show resolved Hide resolved
@julianocosta89
Copy link
Member

I didn't mark the PR as need work because I'm will be OoO for some time, but went through all files and added my review!

This was a really massive one 😅

@julianocosta89
Copy link
Member

Ah, btw...
The loadgen is failing because the FE url is different now.
Maybe we could consider either moving from 8081 to 8080 or updating the loadgen endpoint as well.

@cartersocha
Copy link
Contributor

@wph95 , @cedricziel , @mic-max , @open-telemetry/demo-approvers - ready for final reviews

.env Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
src/frontend/.eslintrc Show resolved Hide resolved
src/frontend/Dockerfile Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/components/CheckoutForm/CheckoutForm.tsx Outdated Show resolved Hide resolved
src/frontend/components/Footer/Footer.tsx Outdated Show resolved Hide resolved
src/frontend/next-env.d.ts Outdated Show resolved Hide resolved
@xoscar
Copy link
Contributor Author

xoscar commented Aug 8, 2022

Hey @mic-max thanks for reviewing the PR 😁 I just updated it based on your changes, let me know if that's good enough to merge the changes. @cartersocha I think we need to press the button one last time 😄

CHANGELOG.md Outdated Show resolved Hide resolved
@reyang
Copy link
Member

reyang commented Aug 8, 2022

@wph95 , @cedricziel , @mic-max , @open-telemetry/demo-approvers - ready for final reviews

I don't know how to effectively review any PR that has more than a hundred files. Maybe it should be changed to a series of smaller PRs?

.env Outdated Show resolved Hide resolved
@xoscar
Copy link
Contributor Author

xoscar commented Aug 9, 2022

@wph95 , @cedricziel , @mic-max , @open-telemetry/demo-approvers - ready for final reviews

I don't know how to effectively review any PR that has more than a hundred files. Maybe it should be changed to a series of smaller PRs?

The idea of one big PR was to not have intermediate commits where the frontend might be broken, this PR adds all of the features the GO version used to have plus the new redesigns.

The next ones should be fairly small as we will only focus on one section like:

  1. Instrumentation.
  2. E2E tests.

CHANGELOG.md Outdated Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
src/frontend/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jorgeepc jorgeepc left a comment

Choose a reason for hiding this comment

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

Still reviewing it 👀

src/frontend/components/CheckoutForm/CheckoutForm.tsx Outdated Show resolved Hide resolved
src/frontend/components/Footer/Footer.tsx Outdated Show resolved Hide resolved
src/frontend/components/Footer/Footer.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@jorgeepc jorgeepc left a comment

Choose a reason for hiding this comment

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

Great job @xoscar

src/frontend/package.json Show resolved Hide resolved
@cartersocha cartersocha merged commit d168790 into open-telemetry:main Aug 9, 2022
@cartersocha
Copy link
Contributor

Thanks @xoscar! Huge contribution 🌟

@puckpuck
Copy link
Contributor

Thank you @xoscar for this!!! 💯

@xoscar xoscar deleted the 39-web-application branch August 10, 2022 00:45
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
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.

Restyle and update product catalog, frontend, etc.
10 participants