Skip to content

Conversation

@gurkanguray
Copy link

@gurkanguray gurkanguray commented Aug 17, 2023

Description

  • Dockerfile and docker-compose.dev.yaml added to the root.
  • Single docker file, multi-target approach applied.
  • Workflow file added for docker checks.
  • Readme file updated with the new docker setup.
  • Sharp added as a new dependency. (ref: https://nextjs.org/docs/messages/install-sharp)

Checklist

  • discord username: gguray
  • Closes feat(ci/cd): setup docker #683
  • PR must be created for an issue from issues under "In progress" column from our project board.
  • A descriptive and understandable title: The PR title should clearly describe the nature and purpose of the changes. The PR title should be the first thing displayed when the PR is opened. And it should follow the semantic commit rules, and should include the app/package/service name in the title. For example, a title like "docs(@kampus-apps/pano): Add README.md" can be used.
  • Related file selection: Only relevant files should be touched and no other files should be affected.
  • I ran npx turbo run at the root of the repository, and build was successful.
  • I installed the npm packages using npm install --save-exact <package> so my package is pinned to a specific npm version. Leave empty if no package was installed. Leave empty if no package was installed with this PR.

How were these changes tested?

Tested on local and GitHub workflow file added.

@gurkanguray gurkanguray requested a review from a team as a code owner August 17, 2023 00:01
@gurkanguray gurkanguray requested review from rasitds and tw4 August 17, 2023 00:01
@vercel
Copy link

vercel bot commented Aug 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
kampus-studio ⬜️ Ignored (Inspect) Visit Preview Aug 23, 2023 3:57am

@vercel
Copy link

vercel bot commented Aug 17, 2023

@gurkanguray is attempting to deploy a commit to the kamp-us Team on Vercel.

A member of the Team first needs to authorize it.

@usirin
Copy link
Member

usirin commented Aug 17, 2023

TY so much for this. I'll take a look ASAP.

@usirin usirin requested review from sevilayerkan and usirin and removed request for tw4 August 17, 2023 22:23
@@ -0,0 +1,13 @@
NODE_ENV=development
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason we have an example env at the root. Since we have examples per folder that's good to move there where it matches

Copy link
Author

Choose a reason for hiding this comment

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

It was for the GitHub workflow testing purposes, and I will remove it in this PR.
https://github.com/marketplace/actions/create-env-file will handle the .env file creation at GitHub workflow for deployment purposes

RUN npm install
COPY --from=gql-prune /app/out/full/ .
COPY --from=gql-prune /app/out/full/apps/gql/.env.example ./apps/gql/.env
COPY .env ./apps/pasaport/.env
Copy link
Contributor

Choose a reason for hiding this comment

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

@usirin should we make a small script to do this on local as well seems like right now .env.example files can run the applications so local env can utilise this too.

Copy link
Member

Choose a reason for hiding this comment

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

from what i read from the next.js docs, the best practice is to switch to a .env.development file and commit that. then people can create a .env.local file if they want to override anything from that files.

Good to know: .env, .env.development, and .env.production files should be included in your repository as they define defaults. .env*.local should be added to .gitignore, as those files are intended to be ignored. .env.local is where secrets can be stored.
https://nextjs.org/docs/app/building-your-application/configuring/environment-variables#default-environment-variables

RUN npm i -g concurrently

# keep running
CMD ["concurrently", "local-ssl-proxy --target 3000 --source 5000", "local-ssl-proxy --target 3001 --source 5001", "local-ssl-proxy --target 4000 --source 5002"]
Copy link
Contributor

Choose a reason for hiding this comment

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

can turbo run these ? turbo has concurrent running capability

Copy link
Member

Choose a reason for hiding this comment

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

at this point we don't need turbo, we pruned everything, and i am not even sure if there is a turbo binary at this point. i think it's better to go with this.

container_name: mysql
image: mysql
command: --default-authentication-plugin=mysql_native_password --skip-warnings
image: mysql:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd say let's define a version so everytime it's the same mysql.

Comment on lines +59 to +63
# healthcheck: # TODO: Add healthcheck for kampus
# test: [ "CMD", "curl", "-f", "http://localhost:3000" ]
# interval: 30s
# timeout: 10s
# retries: 3
Copy link
Contributor

Choose a reason for hiding this comment

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

if not used please remove

"prettier-plugin-tailwindcss": "0.3.0",
"react": "18.2.0",
"react-dom": "18.2.0",
"sharp": "^0.32.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not exact version and why we need this ?

Copy link
Author

Choose a reason for hiding this comment

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

It was added according to the following doc: https://nextjs.org/docs/messages/install-sharp. However, when I researched it more(https://nextjs.org/docs/pages/building-your-application/optimizing/images), this package is needed for built-in image optimizations, not docker images. It is a misunderstanding by me, and I will remove it 🙂

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.

feat(ci/cd): setup docker

3 participants