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.

Sign up for GitHub

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

Jump to bottom

Move GraphBuilder and all its logic from ddr #37

Merged
merged 3 commits into from
Oct 17, 2023
Merged

Move GraphBuilder and all its logic from ddr #37

merged 3 commits into from
Oct 17, 2023

Conversation

c71n93
Copy link
Member

@c71n93 c71n93 commented Oct 16, 2023 •
edited by pr-codex bot
Loading

Closes #34

PR-Codex overview

This PR focuses on updating dependencies and refactoring package structure in the codebase.

Detailed summary

  • Deleted GraphBuilder.ktpom.xml file
  • Added commons-lang3 dependency
  • Updated package imports for GraphBuilder and related classes
  • Updated package imports for test classes
  • Updated package imports for classes related to attributes and nodes
  • Refactored package structure for DeogGraph and Nodes classes
  • Updated package imports for AttributesSetter, ClosedCycleProcessor, InnerPropagator, and CondAttributesSetter classes

The following files were skipped due to too many changes: src/main/kotlin/org/objectionary/deog/steps/CondAttributesSetter.kt, src/main/kotlin/org/objectionary/deog/AttributesUtil.kt, src/main/kotlin/org/objectionary/deog/graph/GraphBuilder.kt

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@c71n93
Copy link
Member Author

c71n93 commented Oct 16, 2023

@Graur could you check this one, please?

Graur
Graur suggested changes Oct 17, 2023
View reviewed changes
src/main/kotlin/org/objectionary/deog/steps/CondAttributesSetter.kt Outdated
val dgCond = DgNodeCondition(cond)
traverseParents(node.parentNode, dgCond.freeVars)
name(node)?.let { name ->
val igCond = DgNodeCondition(cond)
Copy link

Choose a reason for hiding this comment

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

@c71n93 Can you please make this variable name more readable?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Graur done

@c71n93 c71n93 requested a review from Graur October 17, 2023 14:45
Graur
Graur approved these changes Oct 17, 2023
View reviewed changes
Copy link

@Graur Graur left a comment

Choose a reason for hiding this comment

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

@c71n93 Looks good to me

@c71n93
Copy link
Member Author

c71n93 commented Oct 17, 2023

@yegor256 please take a look

@yegor256
Copy link
Member

@rultor merge

@rultor
Copy link
Contributor

rultor commented Oct 17, 2023

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 46c3534 into objectionary:master Oct 17, 2023
@rultor
Copy link
Contributor

rultor commented Oct 17, 2023

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 4min)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Graur Graur Graur approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GraphBuilder.kt:40-41: Refactor this class. Current...
5 participants
@c71n93 @yegor256 @rultor @0pdd @Graur