-
Notifications
You must be signed in to change notification settings - Fork 41
Patch virtual classes regression bug, attribute comment parsing bug, initial config load bug, and add issue templates #338
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
Conversation
…roperly, and add GH issues templates.
Patch virtual classes regression bug, attribute comment parsing bug, and add issue templates
Remove blank issue option
Add placeholder for when community engagement policy is ready to share
@guzman-raphael , I've installed 3.4.1 from your fork, tested the fix and here confirming that it has resolved issue #336. |
Fix issue with double colon attribute comment parsing and dj.config initial load
@guzman-raphael , installed and tested 3.4.2, confirming here that issue #335 is resolved |
Update regex to be comparible to dj-python equivalent
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.
Very nice. I have reviewed and approve but I prefer to replace most string concatenations with sprintf for clearer formatting.
+dj/Computed.m
Outdated
classdef Computed < dj.internal.AutoPopulate | ||
% defines a computed table | ||
properties(Constant) | ||
tierRegexp = ['(?<computed>' ... |
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.
I think sprintf
would be clearer than these types of concatenations.
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.
@dimitri-yatsenko Updated styling per your feedback.
+dj/Imported.m
Outdated
classdef Imported < dj.internal.AutoPopulate | ||
% defines an imported table | ||
properties(Constant) | ||
tierRegexp = ['(?<imported>' ... |
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.
use sprintf
for formatted strings.
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.
Addressed.
getSchema()
for existing schema withdj.Imported
(and likelydj.Computed
) tables #336 issue of instantiating classes with abstract methods (dj.Imported
,dj.Computed
) by using super classdj.internal.UserRelation
and parsing table name to determine table type. This use case is triggered when using the virtual class operation. Addeddj.ERD.getTier
utility method.attribute
's type - incorrectly read from the comments #335 by properly escaping"
and/
characters in attribute comments.This resolves the blocker that @ttngu207 reported related to Moser. Thinking to release
3.4.1
as introduced here and rename milestone from3.4.1
->3.4.2
if others agree.Fix #336
Fix #210
Fix #335