-
-
Notifications
You must be signed in to change notification settings - Fork 156
fix(concerto-analysis): inheritance bug solved in /packages/concerto-analysis related to i/1067 raised #1071
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
… child Signed-off-by: rnema19 <[email protected]>
|
@rnema19 Could you update the PR title to follow commit conventions please? https://www.conventionalcommits.org/en/v1.0.0/ |
Sure, I'd be doing it immediately. |
|
@rnema19 could you also add test cases for the fix? |
| return; | ||
| } | ||
| this.compareProperties(comparers, a.getOwnProperties(), b.getOwnProperties()); | ||
| let propsA = a.getOwnProperties(); |
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.
Wouldn't getProperties() return a super set of getOwnProperties()?
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.
Yes, it would, I'm working on getting exclusive inherited properties.
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.
Is it fine if we use getAddedProperties to check additional or inherited properties, this way we could get the inherited properties set only?
| this.compareProperties(comparers, a.getOwnProperties(), b.getOwnProperties()); | ||
| let propsA = a.getOwnProperties(); | ||
| let propsB = b.getOwnProperties(); | ||
| if (a.getProperties()) { |
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'm guessing this will at least return an empty array, so this condition will always be true.
|
This PR is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Closes Issue#1067
Changes
Flags
Code
Images
Results and Tests
npm run test, more than 96% patching found.Related Issues
Author Checklist
--signoffoption of git commit.mainfromfork:branchname