chore: lint all templates to fix indentation and update style by wrenix · Pull Request #465 · nextcloud/helm

wrenix · 2023-11-11T14:03:39Z

Pull Request

Description of the change

replace indent with nindent (and spaces)
array of yaml indent
with+if replace with with (with is already an if)
nginx if over all probes
use more with
default "just" in values.yaml

Benefits

readable code

Possible drawbacks

none

Applicable issues

none

Additional information

DCO has been signed off on the commit.
Chart version bumped in Chart.yaml according to semver.
(optional) Variables are documented in the README.md

charts/nextcloud/values.yaml

charts/nextcloud/templates/db-secret.yaml

charts/nextcloud/templates/deployment.yaml

jessebot

Left a few comments, but overall good linting/formatting changes :) What did you use to do this and in which text editor/IDE?

jessebot · 2023-11-11T14:44:27Z

Can you also bump this a patch version in the Chart.yaml? I know it's just linting and formatting, but it's always helpful, just in case.

wrenix · 2023-11-11T14:45:19Z

sadly i do that handly and use the https://helix-editor.com/ - but i see that not readible code very often (and start sometimes with some cleanups before i work there)

PS: i am thinking of createn an PR, to manage nextcoud-apps and there configuration with an initcontainer and script using occ-commands ...

jessebot · 2023-11-11T14:48:54Z

sadly i do that handly and use the https://helix-editor.com/ - but i see that not readible code very often (and start sometimes with some cleanups before i work there)

well you did a good job! also that editor looks neat :)

PS: i am thinking of createn an PR, to manage nextcoud-apps and there configuration with an initcontainer and script using occ-commands ...

please see this comment as apparently we already have a post install task mechanism here?:
#272 (comment)

update: just realized you're in that thread too 😄

It would be great to get some docs on that though!

wrenix · 2023-11-11T15:20:07Z

so i hope for the first step, i have everything solved.

PS: I create a discussion issue for my idea of application management, see #466

jessebot

This looks good to me and I learned some stuff along the way, which is always awesome! Due to the size of this PR, I'm going to tag in a couple of other collaborators to review this and make sure I didn't miss anything.

charts/nextcloud/Chart.yaml

Signed-off-by: WrenIX <dev.github@wrenix.eu>

Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

provokateurin

LGTM, just some whitespace nitpicks

charts/nextcloud/templates/deployment.yaml

Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

DanishVaid · 2023-11-18T22:52:29Z

@wrenix I think this introduced a regression. The change in charts/nextcloud/templates/config.yaml line 14 from indent -> nindent causes an extra newline and that leads to a PHP error:

Config file has leading content, please remove everything before "<?php" in custom.config.php
Fatal error: Uncaught Error: Typed static property OC::$server must not be accessed before initialization in /var/www/html/index.php:71 Stack trace: #0 {main} thrown in /var/www/html/index.php on line 71

Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

Signed-off-by: WrenIX <dev.github@wrenix.eu>

…xtraVolumes) Signed-off-by: WrenIX <dev.github@wrenix.eu>

…abels) (#476) * fix: after #465 (remove before <?php and correct indent of extraVolumes) Signed-off-by: WrenIX <dev.github@wrenix.eu> * fix: podLabel inside of with Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com> --------- Signed-off-by: WrenIX <dev.github@wrenix.eu> Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

wrenix force-pushed the chore/cleanup branch 4 times, most recently from cf703bf to 0c1ada9 Compare November 11, 2023 14:12

jessebot reviewed Nov 11, 2023

View reviewed changes

charts/nextcloud/values.yaml Show resolved Hide resolved

jessebot reviewed Nov 11, 2023

View reviewed changes

charts/nextcloud/templates/db-secret.yaml Outdated Show resolved Hide resolved

jessebot reviewed Nov 11, 2023

View reviewed changes

charts/nextcloud/templates/deployment.yaml Show resolved Hide resolved

jessebot reviewed Nov 11, 2023

View reviewed changes

wrenix force-pushed the chore/cleanup branch from c0a1fb2 to 113404d Compare November 11, 2023 15:18

jessebot approved these changes Nov 13, 2023

View reviewed changes

jessebot requested review from provokateurin and tvories November 13, 2023 13:02

jessebot reviewed Nov 13, 2023

View reviewed changes

charts/nextcloud/Chart.yaml Outdated Show resolved Hide resolved

wrenix and others added 10 commits November 14, 2023 00:38

chore: cleanup deployment.yaml

f645c40

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: cleanup config.yaml

977ba5f

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: cleanup db-secret.yaml

df1a32e

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: revert db-secret.yaml stringData

712aab1

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: cleanup _helper.yaml for database

4fd8523

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: cleanup nextcloud-key to default

44bf2bb

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: put know string into printf

c6d2ccf

Signed-off-by: WrenIX <dev.github@wrenix.eu>

fix: dump version after several cleanups

097619e

Signed-off-by: WrenIX <dev.github@wrenix.eu>

chore: fix end after cleanup

a83c707

Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

chore: add some comments for helm-end

60b6047

Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

wrenix force-pushed the chore/cleanup branch from 3087a90 to 60b6047 Compare November 13, 2023 23:39

provokateurin approved these changes Nov 14, 2023

View reviewed changes

chore: remove spaces inside of golang template

befc166

Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

wrenix force-pushed the chore/cleanup branch from eea058e to befc166 Compare November 14, 2023 07:04

jessebot approved these changes Nov 18, 2023

View reviewed changes

jessebot changed the title ~~chore: cleanup deployment.yaml~~ chore: lint all templates to fix indentation and update style Nov 18, 2023

jessebot merged commit a303a5f into nextcloud:main Nov 18, 2023

wrenix deleted the chore/cleanup branch November 18, 2023 22:44

DanishVaid mentioned this pull request Nov 18, 2023

[bug] extra newline in config files #475

Closed

wrenix added a commit to wrenix/nextcloud-helm that referenced this pull request Nov 19, 2023

fix: first line in value of config.yaml after nextcloud#465

16c9c00

Signed-off-by: WrenIX <133280015+wrenix@users.noreply.github.com>

wrenix mentioned this pull request Nov 19, 2023

fix: after #465 (remove before <?php, indent of extraVolumes and podLabels) #476

Merged

3 tasks

wrenix added a commit to wrenix/nextcloud-helm that referenced this pull request Nov 19, 2023

fix: first line in value of config.yaml after nextcloud#465

3775486

Signed-off-by: WrenIX <dev.github@wrenix.eu>

wrenix added a commit to wrenix/nextcloud-helm that referenced this pull request Nov 19, 2023

fix: first line in value of config.yaml after nextcloud#465

ccb18af

Signed-off-by: WrenIX <dev.github@wrenix.eu>

wrenix added a commit to wrenix/nextcloud-helm that referenced this pull request Nov 19, 2023

fix: first line in value of config.yaml after nextcloud#465

533049d

Signed-off-by: WrenIX <dev.github@wrenix.eu>

wrenix mentioned this pull request Nov 19, 2023

fix: another cleanup and linting (contains move to configfiles) #478

Closed

2 tasks

wrenix added a commit to wrenix/nextcloud-helm that referenced this pull request Nov 19, 2023

fix: after nextcloud#465 (remove before <?php and correct indent of e…

6875a38

…xtraVolumes) Signed-off-by: WrenIX <dev.github@wrenix.eu>

This was referenced Nov 20, 2023

Allow NGINX additional config #393

Merged

Tweak list formatting #398

Closed

Conversation

wrenix commented Nov 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jessebot left a comment

Choose a reason for hiding this comment

Uh oh!

jessebot commented Nov 11, 2023

Uh oh!

wrenix commented Nov 11, 2023

Uh oh!

jessebot commented Nov 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wrenix commented Nov 11, 2023

Uh oh!

jessebot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

provokateurin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DanishVaid commented Nov 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wrenix commented Nov 11, 2023 •

edited

Loading

jessebot commented Nov 11, 2023 •

edited

Loading

DanishVaid commented Nov 18, 2023 •

edited

Loading