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

Sync changes from master (dev/imageResizer) #1421

Conversation

arjunbalgovind
Copy link
Contributor

Summary of the Pull Request

This PR pulls the latest changes from master to the dev/imageResizer branch.

PR Checklist

Validation Steps Performed

  • Built release x64 and installed with MSIX to verify all PowerToys were working. Also ran tests.

Note: After installing with MSIX explorer.exe will have to be restarted from Task Manager or you have to reboot/sign out for the shell extensions to get loaded. This is because of the change to the MSIX installer in master and this is tracked in #1326

alekhyakommuru and others added 30 commits January 14, 2020 15:00
…ion instead which returns a wstring typecast into wchar*
microsoft#1122)

* removed hardcoded strings from CanvasEditorWindow.xaml

* removed hardcoded strings from GridEditorWindow.xaml

* loc

* Localized MainWindow

* reverting MainWindow.xaml as it is not rendering the window as expected

* Changed the resource settings from internal to public

* the culture is set based on the culture of the system UI set in the system settings

* Removed the french resource files used for testing

* Localized canvasWindow and mainwindow

* Removed setting the UI culture explicitly as it would be implicitly set to the culture of system UI

* Removed redundant header file
* Localized shortcut_guide.cpp

* localized overlay_window.cpp

* formatting changes

* Localize overlay window

* removed the README link from the set of localized resources

* Typo: changed upper to lower
* localized dllmain.cpp  of fancyzones project

* localized FancyZones.cpp

* format fancyzones.rc file

* Moved SuperFancyZones back to being a string instead of having it in the resource file as it is the window class name

* reverted changes for window name

* Formatted fancyzones rc file
@arjunbalgovind arjunbalgovind requested a review from a team March 3, 2020 18:15
@arjunbalgovind
Copy link
Contributor Author

@ryanbodrug-microsoft while merging the changes from master I found that the FancyZonesEditor uses the System.Text.Json package whereas in ImageResizer I am currently using Newtonsoft.Json. Should I add a task to migrate to that as well, since otherwise both dlls would be packaged during installation? I don't think this is a 0.16 blocker but it would be code improvement I suppose. The System.Text.Json docs shows that it isn't available for .NET framework though, but it seems to be working for FZ.

* Getting ready for v0.15 update for readme.

* getting ready for 0.15

* spelling tweak

* filled in update section

* fixed spelling mistakes

* updating to what POR is

* Update README.md

* Update README.md

* adding back in MSI

* getting readme ready for 0.15

* tweaks

* adding oss to oss links

* fixing links

* tweaking file names

* Update README.md

* Update README.md

* Update README.md

fixing typo

* Update README.md
@ryanbodrug-microsoft
Copy link
Contributor

@ryanbodrug-microsoft while merging the changes from master I found that the FancyZonesEditor uses the System.Text.Json package whereas in ImageResizer I am currently using Newtonsoft.Json. Should I add a task to migrate to that as well, since otherwise both dlls would be packaged during installation? I don't think this is a 0.16 blocker but it would be code improvement I suppose. The System.Text.Json docs shows that it isn't available for .NET framework though, but it seems to be working for FZ.

@arjunbalgovind . FancyZonesEditor and ImageResizers would be running in separate processes correct? There are advantages to using System.Text.Json, but I don't think there are any issues with sticking with Newtonsoft.Json for now. As you said this shouldn't be a .16 blocker. Feel free to create a github issue to migrate newtonsoftjson to System.Text.Json and take it as a code improvement.

Copy link
Contributor

@ryanbodrug-microsoft ryanbodrug-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@arjunbalgovind
Copy link
Contributor Author

@ryanbodrug-microsoft yes they run in independent processes. I'll make an issue for it

@arjunbalgovind
Copy link
Contributor Author

@ryanbodrug-microsoft I'm planning to squash and merge this PR as I cannot rebase it automatically from GitHub. Manually rebasing using the command line is making me individually fix conflicts in each of the 100 commits. @udit3333 and I did some further digging and we found that rebase & merge is causing more issues than solutions. Even in this PR the commits and files changed in my previous sync which were rebased are appearing here again.

I made a duplicate of this branch, the feature branch and master and tried doing squashes to see its effect. From what I could find after merging the master changes in current branch into the feature branch with squash & merge, the "X commits behind master" which is shown in GitHub doesn't change. However, when creating the final PR from the feature branch to master, after a small merge conflict of 1 or 2 files with git merge master, the diff no longer shows the commits from master. However the commits which I had earlier added during my last sync with master which were rebased are still visible in the diff. Which means even if I squash this and resolve the issue for this PR, during the final PR duplicate commits will appear if I do a normal merge because of the earlier rebase done in #1195.
I guess the only workaround to do that is if there is a way to squash a specific set of commits and maintain the rest of the history.

@arjunbalgovind arjunbalgovind merged commit 1cc3833 into microsoft:dev/imageResizer Mar 3, 2020
@arjunbalgovind
Copy link
Contributor Author

@ryanbodrug-microsoft merged this normally with help from @crutkas. I think it's better if we stick to normal merges whenever we need it for PRs which are just for syncing with master so that the commit history is intact.

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.