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 manual-install Compose.yml: Simplify Environment Variables #5459

Merged
merged 11 commits into from
Oct 24, 2024

Conversation

szaimen
Copy link
Collaborator

@szaimen szaimen commented Oct 22, 2024

Local copy of #5393

flll and others added 7 commits October 8, 2024 11:09
- Removed explicit values for environment variables in `docker-compose.yml`.
- Utilized default values for better flexibility and maintainability.
- Updated network configuration to use the default bridge driver.

Note: Using `network: default` is sufficient within Docker Compose; there's no need to create a separate `nextcloud-network` for all hosts. 🚀


Signed-off-by: lll <2844835+flll@users.noreply.github.com>
Co-authored-by: Jean-Yves <7360784+docjyJ@users.noreply.github.com>
Signed-off-by: lll <2844835+flll@users.noreply.github.com>
Co-authored-by: Jean-Yves <7360784+docjyJ@users.noreply.github.com>
Signed-off-by: lll <2844835+flll@users.noreply.github.com>
`update-yaml.sh`の処理の追加を
- .services[].networks の削除
- =%%を削除する特殊な処理

これにより、
すべてのサービスをdefaultのネットワークに接続させることができ、
変数の簡略化が見込めます

Signed-off-by: lll <2844835+flll@users.noreply.github.com>
Signed-off-by: lll <2844835+flll@users.noreply.github.com>
Co-Authored-By: Jean-Yves <7360784+docjyJ@users.noreply.github.com>
@szaimen szaimen added 2. developing Work in progress enhancement New feature or request labels Oct 22, 2024
@szaimen szaimen changed the title Enh/5393/local branch Refactor manual-install Compose.yml: Simplify Environment Variables Oct 22, 2024
@flll flll marked this pull request as ready for review October 22, 2024 09:43
Signed-off-by: lll <2844835+flll@users.noreply.github.com>
@szaimen
Copy link
Collaborator Author

szaimen commented Oct 22, 2024

@flll the script update-helm.sh is currently producing this output: https://github.com/nextcloud/all-in-one/pull/5458/files. It does not look right and needs to be adjusted.

@flll
Copy link
Collaborator

flll commented Oct 22, 2024

I'm sorry, I made a mistake in my operations.
I don't understand GitHub operations at all and don't know what actions I should take.
I've looked at the checks, but I would appreciate any advice you can offer.
I'm very sorry for the inconvenience.

@szaimen
Copy link
Collaborator Author

szaimen commented Oct 22, 2024

All good, no problem! :)

I've restored the change. Now you can go can follow the instructions from #5393 (comment)

@szaimen
Copy link
Collaborator Author

szaimen commented Oct 22, 2024

There is this PR now where you can check the changes that you are doing to this this pr (5459)
https://github.com/nextcloud/all-in-one/pull/5461/files

flll and others added 2 commits October 24, 2024 16:01
- [update-yaml.sh] Change sed command option from -e to -E
- [update-helm.sh] Add check for docker image tag as first argument, exit 2 if missing
- [update-helm.sh] Modify snap command to run with sudo
- [update-helm.sh] Add new sed commands:
  - [sed] Convert `- AB_CD_EFG` to `- AB_CD_EFG=${AB_CD_EFG}`
  - [sed] Apply conversion only to uppercase variables
  - [sed] Exclude NET_RAW, SYS_NICE, NET_ADMIN, and MKNOD capabilities from conversion
This reverts commit 57d03d8.
Copy link
Collaborator Author

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot! 🤗

@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 24, 2024
@szaimen szaimen merged commit d914287 into main Oct 24, 2024
3 of 4 checks passed
@szaimen szaimen deleted the enh/5393/local-branch branch October 24, 2024 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants