Skip to content

Conversation

@d2dyno1
Copy link
Member

@d2dyno1 d2dyno1 commented Jul 15, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Closes #issue...

Validation
How did you test these changes?

  • Did you build the app and test your changes?
@0x5bfa 0x5bfa requested review from 0x5bfa and removed request for 0x5bfa July 15, 2023 14:05
@yaira2
Copy link
Member

yaira2 commented Sep 21, 2023

@d2dyno1 what's the plan for this PR?

@d2dyno1
Copy link
Member Author

d2dyno1 commented Sep 24, 2023

I'm going to continue work on it this Thursday 🙂

@d2dyno1 d2dyno1 marked this pull request as ready for review October 8, 2023 21:34
@d2dyno1
Copy link
Member Author

d2dyno1 commented Oct 8, 2023

Ready for review!

@d2dyno1 d2dyno1 requested a review from 0x5bfa October 8, 2023 21:35
@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Oct 9, 2023
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.

Great! in terms of codebase quality.

I understand as follows

  • Update app UI thread initialization stage
  • Rename ImageModel to Image
  • Add a service for pinning/unpinning
  • System.IO to SystemIO (FYI, I introduced this simpified namespace alias to see all reference to that namespace because this namespace reference in storage class is not favorable)
@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 Oct 17, 2023
@yaira2 yaira2 merged commit 5fc5d32 into files-community:main Oct 17, 2023
@hishitetsu
Copy link
Member

This appears to prevent launch from a cached instance from working. When I try to launch the app again, the window does not appear.

@yaira2
Copy link
Member

yaira2 commented Oct 17, 2023

Should we revert?

@hishitetsu
Copy link
Member

Before we revert, I first want to figure out how to make launching from the cached instance work again.

@hishitetsu
Copy link
Member

Hmmm, even when I get the window to show up, the tabs don't show up properly or the app crashes.
If it is not required for the V3 release, it would be better to revert it once and test it after the V3 release.

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

4 participants