Skip to content
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

[core] core plugins update #82

Merged
merged 9 commits into from
Jun 25, 2024
Merged

[core] core plugins update #82

merged 9 commits into from
Jun 25, 2024

Conversation

yoannmoinet
Copy link
Member

@yoannmoinet yoannmoinet commented Jun 20, 2024

What and why?

Still building up #78 and trying to split it as much as possible to keep the review complexity down.

How?

This one can be summed up as:

  • Some small config updates that were missed in previous PRs.
  • Refactor of the internal plugin mechanism and move some types.
  • Implementation of internal git plugin that adds some git metadata to the build.
  • Update some test helpers.

@yoannmoinet yoannmoinet marked this pull request as ready for review June 24, 2024 09:16
@yoannmoinet yoannmoinet requested a review from a team as a code owner June 24, 2024 09:16
@yoannmoinet yoannmoinet requested review from jakub-g and removed request for a team June 24, 2024 09:16
@elbywan elbywan self-requested a review June 24, 2024 09:19
@yoannmoinet yoannmoinet mentioned this pull request Jun 24, 2024
@datadog-datadog-staging-us1-all

Software Composition Analysis

We found vulnerabilities in the following libraries (compared a038785 against 85330b7):

Copy link
Member

@elbywan elbywan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@@ -2,20 +2,56 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2019-Present Datadog, Inc.

import type { UnpluginOptions } from 'unplugin';
// #imports-injection-marker
Copy link
Member

@elbywan elbywan Jun 24, 2024

Choose a reason for hiding this comment

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

⛏️ small nit: I don't think this would be easy for a new contributor to understand that the code between these 2 markers gets replaced when the yarn cli integrity tool runs so it might be nice to add a comment or something?
Also I could not find in the contributing.md/readme.md a mention about when to run this tool (when adding a plugin?) but maybe it's not meant to be very often

Copy link
Member Author

Choose a reason for hiding this comment

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

Very good point, I'll update the doc(s) and add comments to give more details on this.

@yoannmoinet yoannmoinet merged commit 8d1fcab into master Jun 25, 2024
5 checks passed
@yoannmoinet yoannmoinet deleted the yoann/core-plugins-update branch June 25, 2024 07:58
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.

2 participants