Skip to content

feat: implement a proper BFS when doing IterSubjects/IterResources for recursive nodes#2838

Merged
barakmich merged 2 commits intomainfrom
barakmich/better_recursion
Jan 22, 2026
Merged

feat: implement a proper BFS when doing IterSubjects/IterResources for recursive nodes#2838
barakmich merged 2 commits intomainfrom
barakmich/better_recursion

Conversation

@barakmich
Copy link
Copy Markdown
Contributor

Description

Testing

References

@barakmich barakmich requested a review from a team as a code owner January 21, 2026 22:01
@github-actions github-actions bot added the area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) label Jan 21, 2026
@barakmich barakmich force-pushed the barakmich/better_recursion branch from d73b323 to 3264c51 Compare January 21, 2026 22:07
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 68.38235% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.30%. Comparing base (dbd2687) to head (7fa8efb).

Files with missing lines Patch % Lines
pkg/query/recursive.go 68.39% 33 Missing and 10 partials ⚠️

❌ Your project check has failed because the head coverage (74.30%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2838      +/-   ##
==========================================
- Coverage   74.35%   74.30%   -0.04%     
==========================================
  Files         483      483              
  Lines       56954    57084     +130     
==========================================
+ Hits        42341    42411      +70     
- Misses      11644    11686      +42     
- Partials     2969     2987      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

LGTM, see question

iter := NewFixedIterator(paths...)
recursive := NewRecursiveIterator(iter, "folder", "parent")

ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was wondering about this in other tests too - if we're using fixed iterators, do we need the datastore at all? When I removed it from some of the tests it seemed to work just fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll file a ticket to check and simplify

@tstirrat15 tstirrat15 force-pushed the barakmich/better_recursion branch from 3264c51 to 6d84587 Compare January 22, 2026 16:12
@barakmich barakmich force-pushed the barakmich/better_recursion branch from 6d84587 to 7fa8efb Compare January 22, 2026 18:56
@barakmich barakmich added this pull request to the merge queue Jan 22, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 22, 2026
@barakmich barakmich added this pull request to the merge queue Jan 22, 2026
Merged via the queue into main with commit 11ba410 Jan 22, 2026
43 of 45 checks passed
@barakmich barakmich deleted the barakmich/better_recursion branch January 22, 2026 23:24
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants