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: migrate Vue 2 to 3 #57

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

Conversation

ninoseki
Copy link
Contributor

Migrate Vue 2 to 3 #52

@Fitblip
Copy link
Member

Fitblip commented Dec 12, 2020

Hey @ninoseki, this is great! Huge thanks for putting in the legwork to get this ported over. There's a few auto-formatting things that are sorta wonky, but definitely not a huge deal.

You rock! 🙏 🙇

@Fitblip
Copy link
Member

Fitblip commented Dec 12, 2020

2 quick things:

  • Do you mind removing that frontend/src/assets/logo.png file?
  • Do you mind including a full dist build as well? I know it's generally somewhat frowned upon to include built assets like that, but I think it enables people to clone stuff and not worry about dealing with the tooling to get the frontend working and greatly simplifies the deployment story.

@ninoseki
Copy link
Contributor Author

Do you mind removing that frontend/src/assets/logo.png file?

Sorry, it's my bad. 🙇

Do you mind including a full dist build as well?

I agree with you that including built assets a straightforward solution to skip the frontend building processes even if it is no recommended.
But I guess that most of Certstream users run their instance with Docker.
If so, it makes sense to exclude frontend/dist from the repository.

@ninoseki
Copy link
Contributor Author

Docker's multi-stage builds is a right solution to build the frontend in a single Dockerfile.

# build stage
FROM node:14-alpine as build

WORKDIR /frontend

COPY frontend /frontend

RUN npm install && npm run build && rm -rf node_modules

FROM elixir:1.8-alpine

...

# Copy built assets from the build stage
COPY --from=build /frontend/dist/ /opt/app/frontend/dist/

....

I will add commits to:

  • Exclude frontend/dist from the repository
  • Update Dockerfile to build the frontend by using multi-stage builds

if it makes sense with you.

@ninoseki
Copy link
Contributor Author

@Fitblip What do you think which one is better?

  • Exclude frontend/dist from the repository (and update Dockerfile to build the frontend)
  • Keep frontend/dist the same as before

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.

2 participants