-
Notifications
You must be signed in to change notification settings - Fork 322
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
support bundling via esbuild #2763
Conversation
Overall package sizeSelf size: 4.01 MB Dependency sizes
🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report
@@ Coverage Diff @@
## master #2763 +/- ##
=======================================
Coverage 87.63% 87.63%
=======================================
Files 328 329 +1
Lines 11730 11747 +17
Branches 33 33
=======================================
+ Hits 10279 10295 +16
- Misses 1451 1452 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
33662f0
to
e950783
Compare
94d2cc1
to
2af8199
Compare
ef98ae5
to
0d14b2d
Compare
3abb731
to
d949d6a
Compare
"license": "ISC", | ||
"dependencies": { | ||
"esbuild": "0.16.12", | ||
"express": "^4.16.2" |
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.
These two entries are required for the linter to pass, even though they're never installed. Instead, the dev dep packages available in the root of the repo are used.
I could simply delete the package.json file but I kind of like the scripts section as it provides docs of sorts.
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 agree, and also it would be good in any case at some point to decouple this from the parent dependencies.
@duncanista given that you recently looked into the esbuild problem for serverless deeply, could you help take a look? Maybe it's worth a quick trial as well? |
c3ee5b3
to
76ec565
Compare
Appsec files fix landed: #2913 |
Co-authored-by: Bryan English <bryan.english@datadoghq.com>
76ec565
to
e9f64da
Compare
Will create a PR to corp docs once this is live.
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 other than the comment about checking hasSubscribers
before calling unsubscribe
due to the functionally potentially not being there on early versions of diagnostics_channel.
What does this PR do?
diagnostics_channel
polyfillcurrently checking in some phony appsec fileshopefully graceful fallback if appsec template files are missing #2841 mergesNote: once it lands, please push a commit to delete the empty appsec filesMotivation
Plugin Checklist