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

feature: better topological sorting #149

Merged
merged 2 commits into from
Oct 9, 2023
Merged

Conversation

rustatian
Copy link
Member

@rustatian rustatian commented Oct 9, 2023

Reason for This PR

  • False-positive error when most of the plugins are disabled

Description of Changes

  • Do not take into account (remove immediately) plugins which have non 0 indegree value after toposorting. We know at this point, that there are no edges to them.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

Summary by CodeRabbit

  • Refactor: Updated the resolveEdges function in the Endure struct to provide notifications for disabled plugins post topological sorting, enhancing user awareness.
  • Refactor: Modified the TopologicalSort function in the Graph package to directly update the topologicalOrder field, improving code readability and maintainability.
  • Chore: Adjusted the cron schedule and Go version setup in the .github/workflows/codeql-analysis.yml file, ensuring up-to-date and regular code quality checks.
  • Documentation: Added explanatory comments in the GitHub Actions job workflow, improving clarity for maintainers.

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
@rustatian rustatian added the A-Algorithms Area: Algoritms area, meaning all related to the Algos label Oct 9, 2023
@rustatian rustatian requested a review from wolfy-j October 9, 2023 15:58
@rustatian rustatian self-assigned this Oct 9, 2023
@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2023

Walkthrough

This pull request introduces modifications to the Endure struct and Graph package, enhancing the plugin notification system and refining the topological sorting function. Additionally, it updates the GitHub Actions workflow, improving the job schedule, Go version setup, and adding explanatory comments.

Changes

File Summary
edges.go Updated comments and added a notification for disabled plugins post topological sorting.
graph/toposort.go Altered the TopologicalSort function's return type and updated the topologicalOrder field of the Graph struct.
.github/workflows/codeql-analysis.yml Adjusted the cron schedule, updated Go version setup, and added explanatory comments for certain steps.

🐇

In the land of code, where logic intertwines,

Changes are made, as the moonlight shines.

Graphs are sorted, plugins notified,

In the workflow's rhythm, we confide.

Celebrate the changes, for they bring delight,

To the realm of code, they bring light. 🌙✨


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between f0b1f9b and 99b4d40.
Files selected for processing (2)
  • edges.go (3 hunks)
  • graph/toposort.go (2 hunks)
Files skipped from review due to trivial changes (1)
  • edges.go
Additional comments (Suppressed): 2
graph/toposort.go (2)
  • 3-3: The function signature for TopologicalSort has been changed from returning a bool to returning void. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the removal of the boolean return value does not impact the logic of the code where this function is used.

  • 36-36: The function TopologicalSort no longer checks if the length of g.topologicalOrder is greater than or equal to the length of g.vertices. If this check was important for the function's correctness, consider reintroducing it.

- return len(g.topologicalOrder) >= len(g.vertices)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 99b4d40 and 7a7485c.
Files selected for processing (1)
  • .github/workflows/codeql-analysis.yml (2 hunks)
Files skipped from review due to trivial changes (1)
  • .github/workflows/codeql-analysis.yml

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #149 (7a7485c) into master (f0b1f9b) will increase coverage by 0.30%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
+ Coverage   74.97%   75.27%   +0.30%     
==========================================
  Files          19       19              
  Lines         903      914      +11     
==========================================
+ Hits          677      688      +11     
  Misses        195      195              
  Partials       31       31              
Files Coverage Δ
graph/toposort.go 92.30% <100.00%> (-0.29%) ⬇️
edges.go 90.62% <82.35%> (-2.24%) ⬇️

... and 1 file with indirect coverage changes

@rustatian rustatian merged commit d2ff679 into master Oct 9, 2023
8 checks passed
@rustatian rustatian deleted the chore/better-toposorting branch October 9, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Algorithms Area: Algoritms area, meaning all related to the Algos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant