Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

fix: modernize class inheritance #178

Merged
merged 2 commits into from
Jul 17, 2023
Merged

fix: modernize class inheritance #178

merged 2 commits into from
Jul 17, 2023

Conversation

jimmywarting
Copy link
Contributor

Wanted to modernize it a bit

@jimmywarting jimmywarting requested a review from a team as a code owner July 16, 2023 21:26
@wraithgar wraithgar self-assigned this Jul 17, 2023
@wraithgar
Copy link
Member

This is much more readable! Unfortunately tests don't pass and the tests here are quite fiddly. If you can get the entire refactor working without changing tests it makes it easier to land larger refactors like this. If you can't, we may have to break this up into smaller changes that change both the code and the tests. It's easier to reason about changes to both with smaller iterative changes.

@jimmywarting
Copy link
Contributor Author

yea... trying to figure out what's wrong.
the problem with smaller iterative changes where that some classes extends others and dose something like XYZ.call(this, ...args) and that dose not quite work so well when you received a XYZ can not be constructed without using new keyword.

@jimmywarting
Copy link
Contributor Author

I think it where just that the test threshold did get lower cuz it was using less code. so the uncovered lines covered more %
so i added a simple debug test to one of the test cases

@wraithgar wraithgar changed the title classify fix: modernize class inheritance structure Jul 17, 2023
@wraithgar wraithgar changed the title fix: modernize class inheritance structure fix: modernize class inheritance Jul 17, 2023
@wraithgar
Copy link
Member

I have no idea why Pull Request / Lint Commits is failing but that's our problem to fix, not yours. The PR title itself looks fine to me?

@wraithgar wraithgar merged commit aee9063 into npm:main Jul 17, 2023
22 of 25 checks passed
@wraithgar
Copy link
Member

Cheers @jimmywarting

@github-actions github-actions bot mentioned this pull request Jul 17, 2023
@jimmywarting jimmywarting deleted the classify branch July 17, 2023 22:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants