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

Remove UserSettings From Engine #873

Closed
CharliePoole opened this issue Jan 20, 2021 · 7 comments · Fixed by #1132
Closed

Remove UserSettings From Engine #873

CharliePoole opened this issue Jan 20, 2021 · 7 comments · Fixed by #1132

Comments

@CharliePoole
Copy link
Contributor

In TestCentric/testcentric-gui#580 we agreed that both projects would drop user settings from their engines for compatibility. Handling settings is application-specific. This change is breaking.

@CharliePoole
Copy link
Contributor Author

This is ready to work on but we need to decide how to handle user settings. In the GUI, I handled them by continuing to use the same XML settings file, moving the code out of the engine to the GUI Model. We could do something similar here or we could use a different approach for saving settings. @ChrisMaddock What do you think?

@ChrisMaddock
Copy link
Member

What do we actually save in the settings? I thought it was unused.

@CharliePoole
Copy link
Contributor Author

It's only used indirectly in the console runner - by the RecentFilesService. So I guess RecentFiles is what really needs to be replaced. I would lean towards placing it by somethings simple, preferably by a standard approach used by other apps. Once that's done, then there's no more use of UserSettings.

@CharliePoole CharliePoole self-assigned this Feb 10, 2021
@ChrisMaddock
Copy link
Member

ChrisMaddock commented Feb 13, 2021

What does the console do with RecentFilesService?

I guess it depends on the use-case how we replace it - but I can't particularly think what it would be needed for...

@CharliePoole
Copy link
Contributor Author

Turns out I was wrong... UserSettings and RecentFiles both are only used by their tests.

@CharliePoole
Copy link
Contributor Author

Blocked by #896

@CharliePoole
Copy link
Contributor Author

This was merged to dev-4.0 and closed. I'm reopening it so as to replicate it in main.

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

Successfully merging a pull request may close this issue.

2 participants