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

When we reload a profile, always use the same GUID for it #2542

Merged
merged 1 commit into from
Aug 26, 2019

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

This ensures that settings reload works for profiles w/o GUIDs.

After #2475, we no longer persist the auto-generated GUID for a profile. This means that when settings get reloaded, the profiles without GUIDs will get re-created, and the existing tabs with those profiles will no longer be associated with a profile. That's really bad, and can cause explosions.

This will ensure that we always generate the same GUID for a profile that didn't have one, preventing explosions.

If a person has two profiles with the same name and NO guid, then we'll generate the same GUID for both, then cull the second one when we validate that there should be no duplicate profiles.

References

#2475 caused this regression.

PR Checklist

  This ensures that settings reload works for profiles w/o GUIDs
@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Aug 26, 2019
@zadjii-msft zadjii-msft added this to the Terminal 1908.1 milestone Aug 26, 2019
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Yo dawg, I heard you like GUIDs... So I gave your GUID generator a GUID so that you can GUID while you GUID. 🤣

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 26, 2019
@ghost
Copy link

ghost commented Aug 26, 2019

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 7 hours 52 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@DHowett-MSFT DHowett-MSFT merged commit cffa033 into master Aug 26, 2019
@DHowett-MSFT DHowett-MSFT deleted the dev/migrie/b/reload-no-guid branch August 26, 2019 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants