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

Explain walkdir docstring second example. #55541

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

nathanrboyer
Copy link
Contributor

This is a follow-up documentation PR to #55476. I believe the second example in the walkdir docstring is still unintuitive since the result changes each time. I attempted a simple explanation, but I don't really know what I'm talking about. Hopefully someone else can explain what is happening better. Some additional discussion in this Discourse post.

@LilithHafner
Copy link
Member

walkdir returns a stateful iterator (much like Iterators.Stateful). The relevant paragraph from the Stateful docstring that explains this best to me is

  `Stateful` provides the regular iterator interface. Like other mutable
  iterators (e.g. `Base.Channel`), if iteration is stopped early (e.g. by a
  `break` in a `for` loop), iteration can be resumed from the same spot by
  continuing to iterate over the same iterator object (in contrast, an
  immutable iterator would restart from the beginning).

base/file.jl Outdated Show resolved Hide resolved
Co-authored-by: Lilith Orion Hafner <[email protected]>
@LilithHafner LilithHafner added domain:docs This change adds or pertains to documentation domain:filesystem Underlying file system and functions that use it labels Aug 20, 2024
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

LGTM! Does this make sense & seem ready to merge to you, too?

@nathanrboyer
Copy link
Contributor Author

Yep! It gives the user an expectation for the behavior and a link to dig deeper if desired. Thanks!

@LilithHafner LilithHafner added the status:merge me PR is reviewed. Merge when all tests are passing label Aug 20, 2024
@nathanrboyer
Copy link
Contributor Author

As a funny aside, I spent the last two PRs thinking "LGTM" meant "Let's get to merge" and didn't bother looking it up until just now, but that did result in a pretty fast turnaround for these PRs 😆

@LilithHafner
Copy link
Member

To me, "LGTM"/"looks good to me" means "I would merge this as is".

I'm also a fan of merging PRs reasonably quickly because PRs that sit open for long periods of time become stale, both technically and in the minds of authors and reviewers, which makes them harder to progress on.

@LilithHafner LilithHafner merged commit 31d413e into JuliaLang:master Aug 20, 2024
7 of 10 checks passed
@LilithHafner LilithHafner removed the status:merge me PR is reviewed. Merge when all tests are passing label Aug 20, 2024
@nathanrboyer nathanrboyer deleted the patch-1 branch August 20, 2024 18:17
KristofferC pushed a commit that referenced this pull request Sep 12, 2024
This is a follow-up documentation PR to #55476. I believe the second
example in the `walkdir` docstring is still unintuitive since the result
changes each time. I attempted a simple explanation, but I don't really
know what I'm talking about. Hopefully someone else can explain what is
happening better. Some additional discussion in [this Discourse
post](https://discourse.julialang.org/t/find-all-files-named-findthis-csv-in-nested-subfolders-of-rootfolder/118096).

---------

Co-authored-by: Lilith Orion Hafner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:docs This change adds or pertains to documentation domain:filesystem Underlying file system and functions that use it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants