-
-
Notifications
You must be signed in to change notification settings - Fork 358
feat: refs support pseudo refs #2061
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing this and I see how it can be useful!
I set it back to draft as there are no tests which are strictly required. Is this something you'd want to continue on?
If so, I have some additional notes below.
And after writing them I think that iter_pseudo_refs()
should better be its own iterator to avoid all these max_depth
related changes entirely.
b9e24ea
to
a2ec4e7
Compare
I believe I addressed your concerns. PLMK what you think! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much!
I believe I addressed your concerns. PLMK what you think!
First of all, apologies for not responding to the comment above - I somehow missed it, which might just mean that I archived the email without actually acting on it, which was my intent.
This makes it even more unfortunate that what I wanted to say is along the lines of "I see what you mean, let's stick to keeping the SortedLoosePaths
version", instead of creating a separate iterator.
Would you be willing to roll that back?
If you are out of time, just let me know and I will do it. Please note that I will have to rewrite your commits though as the commit-separation isn't addressed yet. (The 'adapt' commits are missing, and as a rule of thump here, changes to a single crate have to go into their own commits).
Further, it seems like With the current iterator setup it's not super trivial to do in |
If I had a dollar for every time I accidentally archived an email I meant to respond to I'd be retired! Totally fine rolling it back! I won't be able to do so until early next week. Also I see I split the adapt commit into one but not the other. IIRC this broke the commit from compiling. Is that acceptable to have a non compiling commit in the chain? |
Thanks so much!
Yes, that's the trade-off to get changelog entries that fit for the respective crate. Otherwise, say, a breaking change would cause two crates to get a minor version bump, the one that actually broke, and the one that just adapted to the breaking change in a private API, along with changelog entries for both.
No problem, I am looking forward to getting this merged. |
max_depth parameter determines the maximum depth the WalkDir will recurse into. Example values: * 0 -> Returns only the root path with no children. * 1 -> Returns the root path, with children. * 2..n -> Returns the root path, children and {n}-grandchildren
Set max_depth parameter to usize::MAX
Set max_depth parameter to usize::MAX
275536c
to
23d7f6a
Compare
Okay. now I think I have addressed the issues/comments. PLMK what you think! |
Adds support in the references iter Platform to iterate over the pseudo references in the repository.
Note that the
all
funciton will not return pseudo references to maintain backwards compatibility.