Skip to content

Replace ArrayList with List in several designers#10432

Merged
lonitra merged 2 commits into
dotnet:mainfrom
halgab:arraylist-designers
Dec 15, 2023
Merged

Replace ArrayList with List in several designers#10432
lonitra merged 2 commits into
dotnet:mainfrom
halgab:arraylist-designers

Conversation

@halgab
Copy link
Copy Markdown
Contributor

@halgab halgab commented Dec 5, 2023

Contributes to #8140

Proposed changes

Microsoft Reviewers: Open in CodeFlow

@halgab halgab requested a review from a team as a code owner December 5, 2023 20:39
@ghost ghost assigned halgab Dec 5, 2023
List<Control> tempList = new();
tempList.AddRange(_dragControls);

DesignerUtils.CopyDragObjects(tempList, Component.Site);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't this be

Suggested change
DesignerUtils.CopyDragObjects(tempList, Component.Site);
tempList = DesignerUtils.CopyDragObjects(tempList, Component.Site);

?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm.. This does look a bit suspicious, but I am concerned about whether this was intentional and what effects this would have if we do make this change. It's not immediately clear from looking here. Let's open a separate issue on this to investigate.

@elachlan
Copy link
Copy Markdown
Contributor

elachlan commented Dec 5, 2023

Might be overlap with #10279

@halgab
Copy link
Copy Markdown
Contributor Author

halgab commented Dec 13, 2023

@lonitra not sure what the CLA check has been up to for a week here...

Copy link
Copy Markdown
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

👍

List<Control> tempList = new();
tempList.AddRange(_dragControls);

DesignerUtils.CopyDragObjects(tempList, Component.Site);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm.. This does look a bit suspicious, but I am concerned about whether this was intentional and what effects this would have if we do make this change. It's not immediately clear from looking here. Let's open a separate issue on this to investigate.

@lonitra
Copy link
Copy Markdown
Member

lonitra commented Dec 14, 2023

/azp run

@lonitra lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Dec 14, 2023
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@lonitra lonitra merged commit b870b50 into dotnet:main Dec 15, 2023
@ghost ghost added this to the 9.0 Preview1 milestone Dec 15, 2023
@ghost ghost removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Dec 15, 2023
@halgab halgab deleted the arraylist-designers branch December 15, 2023 13:10
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants