Skip to content

Fix OrderedSet example usage#491

Merged
lorentey merged 2 commits intoapple:mainfrom
azarovalex:fix-orderedset-usage
Jul 9, 2025
Merged

Fix OrderedSet example usage#491
lorentey merged 2 commits intoapple:mainfrom
azarovalex:fix-orderedset-usage

Conversation

@azarovalex
Copy link
Copy Markdown
Contributor

@azarovalex azarovalex commented Jun 29, 2025 •
edited
Loading

Code presented as an example usage of OrderedSet doesn't compile since mutating sort function is called on immutable let.

Checklist

  • I've read the Contribution Guidelines
  • My contributions are licensed under the Swift license.
  • I've followed the coding style of the rest of the project.
  • I've added tests covering all new code paths my change adds to the project (if appropriate).
  • I've added benchmarks covering new functionality (if appropriate).
  • I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
  • I've updated the documentation if necessary.

@azarovalex azarovalex requested a review from lorentey as a code owner June 29, 2025 14:59
@xwu
Copy link
Copy Markdown
Contributor

xwu commented Jun 29, 2025

On the other hand, a var binding that's never mutated triggers a warning, and I think it's a fair argument that we shouldn't give examples that lead to warnings :) It doesn't seem like someone who's at the point of their developer journey that they're looking for third-party packages would realistically have fundamental misunderstandings about let vs. var.

@azarovalex
Update OrderedSet.swift
9f68408 
Make values immutable if they are never mutated in example usage docs.
@azarovalex
Copy link
Copy Markdown
Contributor Author

azarovalex commented Jun 30, 2025

On the other hand, a var binding that's never mutated triggers a warning, and I think it's a fair argument that we shouldn't give examples that lead to warnings :) It doesn't seem like someone who's at the point of their developer journey that they're looking for third-party packages would realistically have fundamental misunderstandings about let vs. var.

@xwu agree with your point, fixed this and a few other places where variables are never mutated but defined with var.

lorentey
lorentey approved these changes Jul 3, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

Thanks for auditing these!

@lorentey
Copy link
Copy Markdown
Member

lorentey commented Jul 3, 2025

@swift-ci test

@lorentey lorentey merged commit dcee718 into apple:main Jul 9, 2025
22 of 24 checks passed
@lorentey lorentey added this to the 1.3.0 milestone Sep 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lorentey lorentey lorentey approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@azarovalex @xwu @lorentey