nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.

GNU Affero General Public License v3.0

337 stars 268 forks source link

Added support for Built-in CODE Server (ARM64) #540

Closed dgiebert closed 9 months ago

dgiebert commented 9 months ago

Pull Request

Description of the change

Made the nginx regex also catch the _arm64 for the code server

Additional information

[x] DCO has been signed off on the commit.
[ ] Chart version bumped in Chart.yaml according to semver.

provokateurin commented 9 months ago

Thanks for your contribution, can you link to the code that shows this is necessary? It looks like https://github.com/nextcloud/documentation/blob/e9c1af1ff00625cb24e7e6ccf4d547c15bd714c8/admin_manual/installation/nginx-root.conf.sample#L145 also doesn't have it, could you also contribute it to the general documentation?

dgiebert commented 9 months ago

It's a minor thing. Do you want me to bump the chart version?

provokateurin commented 9 months ago

Yes our CI requires every PR that touches the templates to update the version. You can just bump the patch version though since it is only a bug fix. Can you still link to the code that shows exactly why this change is needed?

dgiebert commented 9 months ago

I will bump the version tomorrow then.

Sadly I cannot find it in code right now, but there are multiple pointers (and ofc I have tested it locally)

See here: https://github.com/CollaboraOnline/richdocumentscode/issues/204#issuecomment-1517211152

Might be related to the app being called different, also see here: https://github.com/CollaboraOnline/richdocumentscode?tab=readme-ov-file#implementation

provokateurin commented 9 months ago

I see, thanks a lot. Only bumping the version is needed, then we can merge this.