Skip to content

Conversation

@ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented May 3, 2019

Moves that global variable to the bundle. Deprecation is not possible right now. See details below.

Details The `oc_appconfig` var can't be deprecated though, as dirty hacks like https://github.com/nextcloud/server/blob/3d42d4c9dc75f0bddda5140835ff0a232768d529/apps/sharebymail/lib/Settings.php#L37-L52 hook into the config array and write `oc_appconfig`. For the deprecation property getter we'd can't have an existing `window.oc_appconfig` property though.

How public is that config hook? What if I just renamed all oc_appconfig occurences to _oc_appconfig in this repo. @MorrisJobke could you maybe check with your local apps if the \OCP\Config::js hook is used anywhere outside server?

If possible I will make window.oc_appconfig a deprecated prop in a follow-up PR.

@MorrisJobke

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst ChristophWurst force-pushed the refactor/oc-appconfig-bundle branch from 6bb116f to 42be4b7 Compare May 6, 2019 11:37
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Change makes sense 👍

@ChristophWurst
Copy link
Member Author

CI fails with MySQL has gone away -> unrelated

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 6, 2019
@skjnldsv skjnldsv merged commit e0c6235 into master May 6, 2019
@skjnldsv skjnldsv deleted the refactor/oc-appconfig-bundle branch May 6, 2019 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants