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

Update Arrays page #3292

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Update Arrays page #3292

merged 1 commit into from
Sep 20, 2023

Conversation

sarahhaggarty
Copy link
Collaborator

@sarahhaggarty sarahhaggarty commented Nov 30, 2022

This PR updates the Arrays page with additional content.

It will resolve the following YouTrack tickets:

  • KT-55214
  • KT-46054
  • KT-46629
  • KT-49167
  • KT-48481
  • KT-50043
  • KT-52564
  • KT-56228
  • KT-57718
  • KT-57473
  • KT-58318
  • KT-58810
  • KWHF-18
  • KWHF-86
  • KWHF-765
  • KWHF-934

Copy link
Member

@SebastianAigner SebastianAigner left a comment

Choose a reason for hiding this comment

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

Some content and structure changes are required here – Arrays are a traditionally tricky subject in Kotlin from a philosophical standpoint, but for more on that, see my detailed comments.

If any of my points (or your changes) directly relate to any of these tickets and contain more reasoning that I somehow overlooked, do let me know, it's a bit hard mapping the individual issues back to all these changes.

docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/collections-overview.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
@sarahhaggarty sarahhaggarty force-pushed the update-basic-types-arrays branch 5 times, most recently from 4c8bd3f to 82a33b1 Compare January 12, 2023 09:04
Copy link
Member

@SebastianAigner SebastianAigner left a comment

Choose a reason for hiding this comment

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

Already looking better! I've added additional comments on your applied changes, as well as additional points that were uncovered by taking a closer look at the current state of the document.

Also, at least for my PRs: Please don't resolve conversations opened by me. I need to validate that the changes you make reflect my original intention, and that I didn't accidentally miscommunicate or that something fell by the wayside. Once the subject of discussion from a conversation has been addressed, I'll make sure to close them for you!

docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/collections-overview.md Outdated Show resolved Hide resolved
@AlexanderPrendota

This comment was marked as resolved.

1 similar comment
@AlexanderPrendota

This comment was marked as duplicate.

@sarahhaggarty sarahhaggarty force-pushed the update-basic-types-arrays branch 2 times, most recently from 34e3065 to 71ed52a Compare January 25, 2023 09:53
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
Copy link
Member

@SebastianAigner SebastianAigner left a comment

Choose a reason for hiding this comment

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

Looks great now! 🎉🎉🎉

@github-actions
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 30, 2023
@SebastianAigner
Copy link
Member

Bump!

@github-actions
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Copy link
Member

@koshachy koshachy left a comment

Choose a reason for hiding this comment

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

Huge work!
I'm so sorry for so late review, but I think it will help make our docs better.

Besides comments I still have a doubt about the document structure.
I tried to create a new one quickly, but we need to think over it:

-- When to use arrays
-- Create arrays
--- via functions
---- regular
---- empty
---- multidimensional
--- using constructor
-- access and modify elements
--- operator[]
--- getters and setters
--- pass variable (isn't it about accessing array elements?)
-- operations with arrays
--- compare
--- transform
--- concatenate
--- convert array to collection
-- primitive-type arrays

Feel free to contact me if you have any questions or would like to discuss the topic further.

docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Show resolved Hide resolved
docs/topics/collections-overview.md Outdated Show resolved Hide resolved
docs/topics/collections-overview.md Outdated Show resolved Hide resolved
docs/topics/arrays.md Outdated Show resolved Hide resolved
@sarahhaggarty
Copy link
Collaborator Author

Thanks so much for your review @koshachy!
I have updated the PR based on your comments. Please can you review again paying particular attention to:

  • Overall docs structure
  • Restructured "When to use arrays" section about collection benefits
  • Restructured "Create arrays" section with empty arrays now included
  • Revamped table for primitive-type arrays (I tried lists before but found it took up too much space)
  • New intro to "Pass variable number of arguments to a function" section

You can find the latest commit in TC preview.

Copy link
Member

@koshachy koshachy left a comment

Choose a reason for hiding this comment

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

Great job!
I hope it will help our reader to understand Arrays in Kotlin better!

@sarahhaggarty sarahhaggarty merged commit 569bd3b into master Sep 20, 2023
4 checks passed
@sarahhaggarty sarahhaggarty deleted the update-basic-types-arrays branch September 20, 2023 16:34
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.

4 participants