Skip to content

Conversation

@AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Jun 1, 2021

Depends on #3631

I am splitting up my changes so that each package bump is reviewed separately. This to to encourage ease of review + split up the changes that could be quite big.

When running madge on @sentry/node:

$ madge --circular src/index.ts
Processed 10 files (698ms) (8 warnings)

✖ Found 1 circular dependency!

1) backend.ts > parsers.ts

To remove the circular relationship between backend and parsers,
we extract the NodeOptions type into a separate types.ts file.

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

@AbhiPrasad AbhiPrasad force-pushed the abhi/fix-circular-dep branch from f682079 to ee593c9 Compare June 1, 2021 16:58
Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AbhiPrasad AbhiPrasad marked this pull request as ready for review June 1, 2021 18:48
@AbhiPrasad AbhiPrasad requested a review from kamilogorek as a code owner June 1, 2021 18:48
@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.83 KB (+0.01% 🔺)
@sentry/browser - Webpack 22.07 KB (0%)
@sentry/react - Webpack 22.11 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.23 KB (-0.01% 🔽)

@AbhiPrasad AbhiPrasad force-pushed the abhi/fix-circular-dep branch from a0adb59 to 43b7e8d Compare June 1, 2021 19:18
Madge (https://www.npmjs.com/package/madge) is a library that can
traverse module dependency trees to check for circular dependencies.

We can leverage it using `madge --circular /path/to/index.ts` to make
sure that our packages don't contain circular deps. On average madge
takes around 2-3 sec, so we are not sacrificing a ton of CI time.
Madge (https://www.npmjs.com/package/madge) is a library that can
traverse module dependency trees to check for circular dependencies.

We can leverage it using `madge --circular /path/to/index.ts` to make
sure that our packages don't contain circular deps. On average madge
takes around 2-3 sec, so we are not sacrificing a ton of CI time.
When running madge on `@sentry/node`:

$ madge --circular src/index.ts
Processed 10 files (698ms) (8 warnings)

✖ Found 1 circular dependency!

1) backend.ts > parsers.ts

To remove the circular relationship between backend and parsers,
we extract the NodeOptions type into a separate types.ts file.
@AbhiPrasad AbhiPrasad force-pushed the abhi/fix-circular-dep branch from 43b7e8d to 765acc1 Compare June 2, 2021 12:45
@AbhiPrasad AbhiPrasad merged commit 2c4f4b8 into abhi/circular-dep Jun 2, 2021
@AbhiPrasad AbhiPrasad deleted the abhi/fix-circular-dep branch June 2, 2021 12:50
AbhiPrasad added a commit that referenced this pull request Jun 2, 2021
When running madge on @sentry/node:

$ madge --circular src/index.ts
Processed 10 files (698ms) (8 warnings)

✖ Found 1 circular dependency!

1) backend.ts > parsers.ts

To remove the circular relationship between backend and parsers,
we extract the NodeOptions type into a separate types.ts file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants