-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Code Quality: Move some ViewModel classes into the appropriate namespaces #12342
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing seems to miss, but still there are some style-related changes that you should consider
@ferrariofilippo I'm currently working on step 1 in #12340. All of them are out of scope of the PR.
|
@ferrariofilippo But thank you always for reviewing my refactoring PR. I will refer to those reviews in the further PR. @yaira2 Ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked it again and nothing is missing, so LGTM!
I found another couple of things that you may change in phase 2:
- At
src/Files.App/ViewModels/UserControls/StatusCenterViewModel.cs
CloseBanner()
, it might be better to useIndexOf
and thenRemoveAt
to avoid comparing all the elements twice - At
src/Files.App/Data/Items/StatusBanner.cs StatusBanner()
, line 85, it might be better to use a swtch assing like:Title = Operation switch ...
Thanks! Can I resolve the conversations? |
Yes |
Description
The first step to clean up ViewModels.
Motivation and Context
PR Checklist
Related Code Quality: Continued work to clean up entire project #12340
Screenshots
None