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

File scoped namespaces #1700

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AndrewKeepCoding
Copy link
Contributor

Description

This PR will switch C# files from block-scoped namespaces to file-scoped namespaces.
Closes #1689.

Motivation and Context

Improves readability.

How Has This Been Tested?

  • Confirmed that all unit tests passes.
  • Run the app and navigated through all pages.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@ghost1372
Copy link
Contributor

FYI @AndrewKeepCoding
Do we need all these commits? Use Visual Studio tools!
Animation

@AndrewKeepCoding
Copy link
Contributor Author

AndrewKeepCoding commented Dec 13, 2024

I did. 🙂
However, I also reviewed all the files to check the changes, which is why there are separate commits. I've seen some cases that VS (or VS Code) doesn't apply this kind of changes correctly.

@Marv51
Copy link
Contributor

Marv51 commented Dec 14, 2024

Are you planning to add a .git-blame-ignore-revs file for this PR?

@AndrewKeepCoding
Copy link
Contributor Author

AndrewKeepCoding commented Dec 16, 2024

@Marv51 You are right. Thanks.
TODO: Update the target commit id at the .git-blame-ignore-revs file.

@niels9001 niels9001 requested a review from karkarl December 18, 2024 21:46
@marcelwgn
Copy link
Contributor

When creating a new UserControl, the namespace is before the usings. Is that behavior also the case for you @AndrewKeepCoding ?

@AndrewKeepCoding
Copy link
Contributor Author

AndrewKeepCoding commented Jan 10, 2025

@marcelwgn No on my side. This is what I get:

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.WindowsRuntime;
using Windows.Foundation;
using Windows.Foundation.Collections;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Controls.Primitives;
using Microsoft.UI.Xaml.Data;
using Microsoft.UI.Xaml.Input;
using Microsoft.UI.Xaml.Media;
using Microsoft.UI.Xaml.Navigation;

// To learn more about WinUI, the WinUI project structure,
// and more about our project templates, see: http://aka.ms/winui-project-info.

namespace WinUIApp5
{
    public sealed partial class UserControl1 : UserControl
    {
        public UserControl1()
        {
            this.InitializeComponent();
        }
    }
}

@AndrewKeepCoding
Copy link
Contributor Author

By the way, the item templates are being changed to file scoped namespaces in v1.7-exp2.

@marcelwgn
Copy link
Contributor

Huh thats very interesting. Would be good to know why the results differ when creating a new UserControl. But that will be fixed with the new templates anyway (though I would have preferred the namespace to preceed usings but thats a different story).

This topic aside, looks like there are some merge conflicts which are not surprising though with these changes though.

@AndrewKeepCoding
Copy link
Contributor Author

AndrewKeepCoding commented Jan 12, 2025

This topic aside, looks like there are some merge conflicts which are not surprising though with these changes though.

@marcelwgn Thanks! I fixed the conflicts.

@@ -0,0 +1,2 @@
# Switch to file-scoped namespaces
5f4ff16b4bad507c09bd180091ebbb2df3872279
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work with regard to the fact that PRs get squashed&merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The commit for this PR, which primarily involves changes in indentation, will be ignored by git-blame. Right?🤔

@niels9001
Copy link
Contributor

/azp run

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.

Switch to file-scoped namespace
6 participants