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.

Sign up for GitHub

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

Jump to bottom

langref: Document destructuring #21627

Merged
merged 3 commits into from
Jan 28, 2025
Merged

langref: Document destructuring #21627

merged 3 commits into from
Jan 28, 2025

Conversation

87flowers
Copy link
Contributor

Resolves #18523.

wooster0
wooster0 reviewed Oct 8, 2024
View reviewed changes
rootbeer
rootbeer reviewed Oct 8, 2024
View reviewed changes
Copy link
Contributor

@rootbeer rootbeer left a comment

Choose a reason for hiding this comment

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

The new text looks reasonable to me (but I'm not enough of an expert to point out if it has subtle problems). However, I am enough of an expert to point out the missing files.

@haferburg
Copy link
Contributor

Thank you for your effort!

Would you mind changing the text a little so that it mentions the word "unpack" somewhere? My thinking is that a new user might learn this way that the word "unpacking" maps to "destructuring" in zig.

@haferburg haferburg mentioned this pull request Dec 10, 2024
Closed
@87flowers
Copy link
Contributor Author

I don't think introducing additional terminology is helpful, especially an ambiguous word like 'unpack', which could also refer to vector unpacking or similar concepts.

@alexrp
Copy link
Member

alexrp commented Jan 26, 2025

cc @mlugg since IIRC you implemented the feature?

mlugg
mlugg requested changes Jan 26, 2025
View reviewed changes
Copy link
Member

@mlugg mlugg left a comment

Choose a reason for hiding this comment

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

Sorry, these are slight nitpicks. Once they're addressed (feel free to just have GitHub commit them and I'll squash-merge this) I'll be happy to merge since it's certainly an improvement on not having this documented.

@87flowers
Copy link
Contributor Author

Applied as suggested, thanks!

@mlugg mlugg merged commit 183bb8b into ziglang:master Jan 28, 2025
9 of 10 checks passed
@87flowers 87flowers deleted the doc-destruct branch January 29, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rootbeer rootbeer rootbeer left review comments

@wooster0 wooster0 wooster0 left review comments

@mlugg mlugg mlugg requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

langref: document destructuring syntax
6 participants
@87flowers @haferburg @alexrp @rootbeer @mlugg @wooster0