-
Notifications
You must be signed in to change notification settings - Fork 8
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
748 using tarjan for entitytree #805
Conversation
Conflicts: app/models/ontology.rb
subclasses.each do |s| | ||
c1, c2 = s.hierarchical_class_names | ||
|
||
child = ontology.entities.where('name = ? OR iri = ?', c1, c1).first.id |
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.
- Here, the indentation is two spaces too far in (until line 17)
- Why not use
first!
(also in next line)? If there is no such entity, you get a better error message withfirst!
.
t.text :name | ||
|
||
t.timestamps | ||
end |
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.
https://github.com/ontohub/ontohub/pull/805/files#Constraints are missing completely. Please add NOT NULL and FOREIGN KEY constraints. Indices are missing as well.
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.
this is done
@corny any signoff-news? |
end | ||
|
||
end | ||
end |
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.
Indentation and vertical spacing is ugly, here.
👍 |
We decided on already merging this in the meeting. |
Using tarjan for entitytree, fixes #748.
Using the Tarjan algorithm for grouping the nodes of the entity trees to handle cycles in the class hierachy. See #748.