Skip to content

Intellicode suggestions#1814

Closed
Jay-o-Way wants to merge 38 commits intomicrosoft:mainfrom
Jay-o-Way:intellicode-suggestions
Closed

Intellicode suggestions#1814
Jay-o-Way wants to merge 38 commits intomicrosoft:mainfrom
Jay-o-Way:intellicode-suggestions

Conversation

@Jay-o-Way
Copy link
Contributor

@Jay-o-Way Jay-o-Way commented Mar 22, 2025

Description

Scrolled through code and applied a large number of suggestions. Zero effect on functionality or looks.
I skipped a few, like "mark as readonly/static/partial"; IDE0305 "ToList"; and something with name convention (capital letter or "_" at the beginning)

Context

Linking #1785
Note: this PR is all within the WinUIGallery project.

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)

@Jay-o-Way Jay-o-Way marked this pull request as ready for review March 22, 2025 19:19
@niels9001
Copy link
Collaborator

Thanks @Jay-o-Way! Review and merging might take a bit more time, since it's pretty huge and we'd want to avoid PRs having conflicts as much as possible :)

@Jay-o-Way
Copy link
Contributor Author

Thanks! Yeah, I understand this is no priority. I was just wondering -

  • Does simplifying the namespace in a CREF matter?
    e.g. /// and <see cref="Page.OnNavigatedFrom"/> events.
  • Is this something I could do for PowerToys too?

@Jay-o-Way Jay-o-Way mentioned this pull request Mar 23, 2025
3 tasks
@Jay-o-Way
Copy link
Contributor Author

Oh boy, how do I fix those conflicts

@Zakariathr22
Copy link
Contributor

Oh boy, how do I fix those conflicts

I’ve been thinking about this as well 💀.

I suggest that you split this PR into multiple PRs, modifying a part of the project each time. Start with the files that are not being modified now and finish with the other files.

I also suggest opening an issue where you outline your plan.

@Jay-o-Way
Copy link
Contributor Author

Review and merging might take a bit more time, since it's pretty huge and we'd want to avoid PRs having conflicts as much as possible

But in reality, the longer a PR is open, the more chances of merge conflicts.

@niels9001
Copy link
Collaborator

Review and merging might take a bit more time, since it's pretty huge and we'd want to avoid PRs having conflicts as much as possible

But in reality, the longer a PR is open, the more chances of merge conflicts.

Hey @Jay-o-Way We've discussed this with the team and we can't review this PR; it's just too big :(. Although most changes look pretty simple, we'd need to make sure we are not breaking anything.

Can we break this up in smaller PRs? Thinking out loud, I can imagine having a separate PR for the Controls folder, Pages folder etc.

niels9001 added a commit that referenced this pull request Apr 2, 2025
<!--- Provide a general summary of your changes in the Title above -->

## Description

Same as #1814, but now in the **Test projects**.

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] 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)

---------

Co-authored-by: Niels Laute <niels.laute@live.nl>
@niels9001 niels9001 closed this Apr 10, 2025
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.

3 participants