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

Move security settings to configuration > settings menu #11972

Closed
wants to merge 2 commits into from

Conversation

hishamco
Copy link
Member

@hishamco hishamco commented Jul 8, 2022

Fixes #11773

@hishamco hishamco requested a review from Piedone July 8, 2022 13:22
@Skrypt
Copy link
Contributor

Skrypt commented Jul 9, 2022

That's just for one module here while there is another PR for that matter already from @CrestApps ??

@MikeAlhayek
Copy link
Member

Yes. I guess this one is only to move that single item while the other one moves all settings

@Skrypt
Copy link
Contributor

Skrypt commented Jul 9, 2022

Partial change already in #11937
Please explain the PR @hishamco

@hishamco
Copy link
Member Author

hishamco commented Jul 9, 2022

@Skrypt this PR is fixed the original issue, that discussed month and half ago, then @CrestApps created another issue that move all the settings

So, I never mind to close this PR if all agreed about all the changes in other PR. But still I see some feedback and discussion in going on. I don't think there's an issue to merge this while we already agreed about it in the OC standup, or we should wait until #11937 is ready

@Piedone
Copy link
Member

Piedone commented Jul 15, 2022

I'll review this PR next week (including checking if we need it).

@Piedone
Copy link
Member

Piedone commented Jul 17, 2022

I'd close this one and keep #11937. While #11773 was first, under the other PR there might be some further refactoring and reorganizing of menu items. So, I wouldn't add more variables to that.

@hishamco hishamco closed this Jul 18, 2022
@hishamco hishamco deleted the hishamco/security-settings-menu branch July 18, 2022 09:30
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.

Move Security Settings into Configuration Section
4 participants