Skip to content

Conversation

@hez2010
Copy link
Member

@hez2010 hez2010 commented Mar 6, 2024

Resolved / Related Issues

  • Moving database to the server project to ensure exact one database connection existing.
  • Bump dependencies except FluentFTP
  • Also generate Microsoft.UI.Content.ContentExternalOutputLink from winmd on-the-fly to avoid breaking changes in CsWinRT
  • Remove Files.Core, this project is not being shared between any other project so I don't see any reason why it should be a separate project than Files.App

Validation
How did you test these changes?

  • Did you build the app and test your changes?

Screenshots (optional)
Add screenshots here.

@hez2010 hez2010 marked this pull request as draft March 6, 2024 14:43
@hez2010 hez2010 changed the title Feature: LiteDB server Feature: LiteDB server and Files.Core removal Mar 6, 2024
@hez2010 hez2010 marked this pull request as ready for review March 6, 2024 15:22
@yaira2 yaira2 force-pushed the main branch 2 times, most recently from edf33f7 to c79b2cb Compare March 10, 2024 02:11
@yaira2 yaira2 requested review from 0x5bfa and d2dyno1 March 10, 2024 14:52
@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Mar 10, 2024
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Looks good overall.

@0x5bfa
Copy link
Member

0x5bfa commented Mar 26, 2024

Maybe Core removal should be separated for the code review.

@yaira2 yaira2 requested a review from 0x5bfa March 31, 2024 15:39
col.Delete(tmp.Id);
}
}
NormalMaxLength = entry.NormalMaxLength,
Copy link
Member

Choose a reason for hiding this comment

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

I assume everything is saved to the DB transparently, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is to convert LayoutPreferencesItem from Files.App to LayoutPreferencesItem from Files.App.Server. We cannot just simply move the LayoutPreferencesItem from Files.App to Files.App.Server as it contains a lot of services related to UI.
And yeah, changes will be saved to the DB.

Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Overall, look good!

@hez2010
Copy link
Member Author

hez2010 commented Apr 3, 2024

PTAL

Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

lgtm code wise. I"m not sure for layout managers changes validation.

@yaira2 yaira2 merged commit a0acba3 into files-community:main Apr 3, 2024
@yaira2 yaira2 changed the title Feature: LiteDB server and Files.Core removal Code Quality: LiteDB server and Files.Core removal Apr 3, 2024
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed ready for review Pull requests that are ready for review labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Pull requests that are approved and ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants