-
-
Notifications
You must be signed in to change notification settings - Fork 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
- fix: removed circular dependencies #1144
- fix: removed circular dependencies #1144
Conversation
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.
Nice! Thanks for improving this.
I left 1 note.
Label added :) |
Done! Thanks a lot for the label :) |
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 looks good to me.
FYI the e2e tests will fail as non-Sentry-member PRs don't have access to Github Secrets, don't worry about that.
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.
LGTM! THanks!
📢 Type of change
📜 Description
I checked for circular dependencies using
madge . --extensions ts,tsx -c --warning
and fixed the issue (moved one interface to its own module)💡 Motivation and Context
This PR started as a fix for #479, but that issue isn't available anymore, but since I found other cyclic dependencies I removed those
💚 How did you test it?
I didn't introduce any major changes, just remade some imports. I just run all the existent tests
📝 Checklist
🔮 Next steps
I am looking forward to someone to merge this and maybe add the
hacktoberfest-accepted
label to it 😄