Skip to content

Conversation

@rw4lll
Copy link
Contributor

@rw4lll rw4lll commented Oct 29, 2024

Description

Introduces new section /frameworks and adds first guide for Laravel PHP framework based on Docker Compose with examples for development and production environments.

Additionally (a topic to discuss) introduces boolean flag "external" for sidebar links to improve the user experience, when dealing with external links to open them in a new tab.

Related issues or tickets

Reviews

  • Technical review
  • Editorial review
  • Product review

@github-actions github-actions bot added hugo Updates related to hugo area/guides labels Oct 29, 2024
@netlify
Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit acf46fb
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/678518372ec7680008b3efde
😎 Deploy Preview https://deploy-preview-21276--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@dvdksn dvdksn left a comment

Choose a reason for hiding this comment

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

Thanks a lot @rw4lll

Is this PR ready for review yet? 🔍

@rw4lll
Copy link
Contributor Author

rw4lll commented Oct 30, 2024

Thanks a lot @rw4lll

Is this PR ready for review yet? 🔍

I hope so 😄 . I've tried to add tags, but it seems that if add a new tag, its not recognized:

tags: [frameworks]
languages: [php]

This will return error like:

2024-10-30 20:00:23 ERROR [tags] Undefined tag: 'frameworks' in /project/content/_index.md
2024-10-30 20:00:23 ERROR [tags] Undefined tag: 'frameworks' in /project/content/guides/frameworks/laravel/_index.md
2024-10-30 20:00:23 ERROR [tags] Undefined tag: 'frameworks' in /project/content/guides/frameworks/laravel/common-questions.md
2024-10-30 20:00:23 ERROR [tags] Undefined tag: 'frameworks' in /project/content/guides/frameworks/laravel/prerequisites.md
2024-10-30 20:00:23 ERROR [tags] Undefined tag: 'frameworks' in /project/content/guides/frameworks/laravel/production-setup.md
2024-10-30 20:00:23 ERROR [tags] Undefined tag: 'frameworks' in /project/content/guides/frameworks/laravel/development-setup.md
2024-10-30 20:00:25 Built in 4527 ms
2024-10-30 20:00:25 Error: error building site: logged 6 error(s)

But if I use any other tag which already exists, it works well. Where can I register new "frameworks" tag or should I use one existing?

@rw4lll
Copy link
Contributor Author

rw4lll commented Oct 31, 2024

@dvdksn added tag support

@rw4lll rw4lll requested a review from dvdksn October 31, 2024 18:23
<ul class="ml-4 space-y-2">
{{- range . }}
<li><a href="{{ .url }}" class="link">{{ .title }}</a></li>
<li><a href="{{ .url }}" {{- if .external }} target="_blank" {{- end }} class="link">{{ .title }}</a></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this and the corresponding front matter property; last week I'd said LGTM but I was recently made aware that target=_blank is a bad pattern. See this issue:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, fixed

Comment on lines +203 to +197
# Install NVM (Node Version Manager) as the www user
RUN export NVM_DIR="$HOME/.nvm" && \
curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.0/install.sh | bash && \
[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" && \
nvm install ${NODE_VERSION} && \
nvm alias default ${NODE_VERSION} && \
nvm use default

# Ensure NVM is available for all future shells
RUN echo 'export NVM_DIR="$HOME/.nvm"' >> /home/www/.bashrc && \
echo '[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"' >> /home/www/.bashrc && \
echo '[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"' >> /home/www/.bashrc
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why you're using NVM here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dvdksn sure, the idea is to use this container as a workspace for laravel local development. It should include at least php cli for running artisan commands and node for assets building. Nvm here helps to switch between node versions easily through the env param.

Comment on lines 283 to 284
UID: ${UID}
GID: ${GID}
Copy link
Contributor

Choose a reason for hiding this comment

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

If GID and UID env vars are unset, this would set UID and GID to empty strings, and thereby override the default 1000 set in the Dockerfile

Comment on lines 303 to 307
- "${POSTGRES_PORT}:5432"
environment:
- POSTGRES_DB=${POSTGRES_DATABASE}
- POSTGRES_USER=${POSTGRES_USERNAME}
- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}
Copy link
Contributor

Choose a reason for hiding this comment

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

This compose file has a lot of dependencies to environment variables. I think it's better to try and keep non-secret values in the compose file directly. Still, the .env will need some explanation.

@rw4lll
Copy link
Contributor Author

rw4lll commented Dec 27, 2024

@dvdksn After the review I've decided to refactor the example repo to make it simpler and optimize it by utilizing multi-stage builds where possible. Imo it should looks better now. So I've updated the guide following the review feedback, as well as the updates from example repo.

@rw4lll rw4lll requested a review from dvdksn December 27, 2024 16:52
Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
dvdksn
dvdksn previously approved these changes Jan 9, 2025
Copy link
Contributor

@dvdksn dvdksn left a comment

Choose a reason for hiding this comment

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

💪🏻💯 I just pushed some formatting/wording updates, and added some terms to our style/vocabulary files. LGTM!

Two things that I'd like to do:

  1. Fork the example repository into the dockersamples org and link there. Would you be OK with that @rw4lll?
  2. Add a note that attributes the guide to you, to thank you for the hard work you put in to create this 🙏🏻 Similar to what we have e.g. here - is that OK with you @rw4lll and if so, where would you want the link to point to?

@rw4lll
Copy link
Contributor Author

rw4lll commented Jan 10, 2025

@dvdksn Thanks, that sounds great! 🎉

  1. Yes, absolutely.
  2. Of course! You can point the link to my GitHub profile. Let me know if there’s anything else you’d need from me!

Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
Copy link
Contributor

@dvdksn dvdksn left a comment

Choose a reason for hiding this comment

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

Thanks @rw4lll

@dvdksn dvdksn merged commit d08fe30 into docker:main Jan 13, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/guides area/tests hugo Updates related to hugo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Laravel + Docker guide

2 participants