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

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

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

dgiebert
Copy link
Contributor

@dgiebert dgiebert commented Feb 28, 2024

Pull Request

Description of the change

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

Additional information

Signed-off-by: Dominic Giebert <dominic.giebert@suse.com>
@provokateurin
Copy link
Member

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
Copy link
Contributor Author

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

@provokateurin
Copy link
Member

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
Copy link
Contributor Author

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: CollaboraOnline/richdocumentscode#204 (comment)

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

@provokateurin
Copy link
Member

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

Signed-off-by: Dominic Giebert <dominic.giebert@suse.com>
@provokateurin provokateurin merged commit 9851531 into nextcloud:main Feb 29, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants