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

Configuration Options for Windows Registry #2914

Merged
merged 2 commits into from
Feb 12, 2021

Conversation

marcohald
Copy link
Contributor

Hi,
I created this issue #2890 and tried to implement it.
I have added these configuration options

  • confirmExternalStorage
  • crashReporter
  • newBigFolderSizeLimit
  • useNewBigFolderSizeLimit

to the Windows registry.
They should behave the same as when you change it in the GUI.
I used a REG File wih these Settings to test:

Windows Registry Editor Version 5.00

[HKEY_CURRENT_USER\Software\Policies\Nextcloud GmbH\Nextcloud]
"newBigFolderSizeLimit"=dword:00005000
"skipUpdateCheck"=dword:00000001
"confirmExternalStorage"=dword:00000000
"useNewBigFolderSizeLimit"=dword:00000000

This is what the Client UI looks like:
grafik

src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
@camilasan
Copy link
Member

camilasan commented Feb 9, 2021

Thanks for creating a feature request and the PR 🎉 for it

@marcohald marcohald requested a review from camilasan February 9, 2021 14:58
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
src/libsync/configfile.cpp Outdated Show resolved Hide resolved
@allexzander allexzander requested a review from er-vin February 11, 2021 07:48
Copy link
Member

@er-vin er-vin left a comment

Choose a reason for hiding this comment

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

LGTM, just the remaining comment from @allexzander to address and it's good to go.

Probably worth backporting too after proper testing.

@marcohald
Copy link
Contributor Author

marcohald commented Feb 12, 2021

Should be fixed now. Thank you all for your great support.
I compiled this into the 3.1.2 Client and deployed it to 50 devices yesterday.
It seems to be working. When somebody can verify this would be great if it gets backported.

@er-vin
Copy link
Member

er-vin commented Feb 12, 2021

OK, I'll rebase it to get rid of the merge commits. Once this is done could you please just pull and squash it all back into a single commit? Then it can go in.

@er-vin
Copy link
Member

er-vin commented Feb 12, 2021

/rebase

@er-vin
Copy link
Member

er-vin commented Feb 12, 2021

Ah damn, the automatic rebase failed because your branch is called master and the script doesn't expect that, this step would need to be done manually as well. Could you please squash into a single commit and rebase on latest master by hand?

We unfortunately can't do it for you and push it ourselves since master is a protected branch in your repository.

Added the configuration  options
        confirmExternalStorage
        crashReporter
        newBigFolderSizeLimit
        useNewBigFolderSizeLimit
    to the Windows registry

Signed-off-by: Marco Hald <marcohald@users.noreply.github.com>
@marcohald
Copy link
Contributor Author

marcohald commented Feb 12, 2021

@er-vin I hope this has worked now. I do not really know how to use git.
I did it with git rebase -i 6be88c6 and manually added the changes back in the file.
I think there is a better a way than that.

@allexzander
Copy link
Contributor

/rebase

@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-2914-43ed8423e5014081bb4b04ca47cb7c80e2d2ea98-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@allexzander
Copy link
Contributor

@marcohald please rebase it against the latest master and feel free to merge.

@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-2914-6032f2baef0d22fcf90664f85e1387cb64c2351f-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@allexzander
Copy link
Contributor

I guess now we just need a go from @camilasan so the merging will become possible.

@camilasan camilasan merged commit 24ab49d into nextcloud:master Feb 12, 2021
@camilasan
Copy link
Member

I did read the conversation but I missed the fact that the merge commit was still there :/

@er-vin
Copy link
Member

er-vin commented Feb 13, 2021

/backport to stable-3.1

@backportbot-nextcloud
Copy link

The backport to stable-3.1 failed. Please do this backport manually.

@er-vin
Copy link
Member

er-vin commented Feb 13, 2021

OK... backport bot totally failed, I guess it's confused by the merge commit. I guess it'll have to wait for 3.2.0, this is a feature not a bug fix after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants