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

feat: getAllDependents & getAllDependencies #41561

Merged
merged 7 commits into from
Nov 23, 2023

Conversation

adripo
Copy link
Contributor

@adripo adripo commented Oct 23, 2023

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues.

Fixes #36113

Approach

In the modified code, I've added two new methods: getAllDependents and getAllDependencies. These methods use recursion to traverse the graph and collect all direct and indirect dependents/dependencies of a given node. The helper methods getAllDependentsRecursive and getAllDependenciesRecursive are used for the recursive traversal.

Check List

  • Read the Contributing Guide
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

@CLAassistant
Copy link

CLAassistant commented Oct 23, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@sameerajayasoma sameerajayasoma left a comment

Choose a reason for hiding this comment

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

This code looks perfect.

Can you please add some unit test cases to validate your implementation to this test class?

Since the DependencyGraph<T> accepts any T, you can use a custom object as the node.

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (3957cf1) 76.70% compared to head (b20b0ea) 76.71%.
Report is 20 commits behind head on master.

Files Patch % Lines
...in/java/io/ballerina/projects/DependencyGraph.java 0.00% 24 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #41561      +/-   ##
============================================
+ Coverage     76.70%   76.71%   +0.01%     
+ Complexity    52693    52689       -4     
============================================
  Files          2879     2879              
  Lines        198656   198680      +24     
  Branches      25809    25813       +4     
============================================
+ Hits         152372   152427      +55     
+ Misses        37868    37824      -44     
- Partials       8416     8429      +13     

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

@adripo adripo requested a review from hevayo as a code owner October 27, 2023 18:26
@adripo
Copy link
Contributor Author

adripo commented Oct 27, 2023

@sameerajayasoma I created the tests as requested. Please re-run the checks.

@adripo
Copy link
Contributor Author

adripo commented Oct 30, 2023

@sameerajayasoma could you also label the PR as hacktoberfest-accepted before 31 october? Thank you

Copy link
Contributor

@sameerajayasoma sameerajayasoma left a comment

Choose a reason for hiding this comment

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

Changes look great!!! Thanks for your contribution.

sameerajayasoma
sameerajayasoma previously approved these changes Nov 2, 2023
Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

@github-actions github-actions bot added the Stale label Nov 22, 2023
@adripo
Copy link
Contributor Author

adripo commented Nov 23, 2023

@github-actions still active

@keizer619 keizer619 removed the Stale label Nov 23, 2023
@azinneera azinneera merged commit 1c44405 into ballerina-platform:master Nov 23, 2023
16 of 17 checks passed
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.

Add getAllDependents and getAllDependencies method to the DependencyGraph class
5 participants