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

[ci-app][CIAPP-936] Add More Git Metadata to Tests #1254

Merged
merged 8 commits into from
Mar 25, 2021

Conversation

juan-fernandez
Copy link
Collaborator

@juan-fernandez juan-fernandez commented Mar 2, 2021

What does this PR do?

  • Add git-repo-info which reads from the .git folder and does not rely on git command.
  • Use git-repo-info to add commit message, author date, name and email, and committer date, name and email to all tests.

Motivation

Be able to show rich commit metadata for clients that do not use our instrumented CIs

Plugin Checklist

Additional Notes

@juan-fernandez juan-fernandez marked this pull request as ready for review March 2, 2021 09:28
@juan-fernandez juan-fernandez requested a review from a team as a code owner March 2, 2021 09:28
rochdev
rochdev previously approved these changes Mar 3, 2021
@juan-fernandez
Copy link
Collaborator Author

@rochdev I'll delay this a bit to see if the maintainer of git-repo-info responds to my PR (rwjblue/git-repo-info#60). There is a bug with multi line commit messages. If there's no response I'll have to figure out a solution.

* copy index.js from repository and adjust
* add file to eslintignore
@juan-fernandez
Copy link
Collaborator Author

@rochdev I've added the single logic file from git-repo-info to the PR in debfe06 as we talked about in Slack. The fix for the multiline commit message is in debfe06#diff-f79e00db382cdd07bca74e601f55ea00c7bfbf34bdf968eaf446d3e779aec710R339-R345.

.eslintignore Outdated
@@ -6,3 +6,4 @@ node_modules
protobuf
versions
acmeair-nodejs
packages/dd-trace/src/plugins/util/git-repo-info.js
Copy link
Member

Choose a reason for hiding this comment

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

Could we move this to a vendor folder instead and ignore the entire folder?

@@ -0,0 +1,356 @@
// From https://github.com/rwjblue/git-repo-info/blob/d3ab418ef8b392eabbe911a37871708b15201b70/index.js
Copy link
Member

Choose a reason for hiding this comment

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

The file should also be added to LICENSE-3rdparty.csv similar to the TDigest entry.

@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #1254 (70b030a) into master (9baa26d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1254      +/-   ##
==========================================
+ Coverage   92.56%   92.58%   +0.02%     
==========================================
  Files         150      150              
  Lines        6095     6113      +18     
==========================================
+ Hits         5642     5660      +18     
  Misses        453      453              
Impacted Files Coverage Δ
packages/dd-trace/src/plugins/util/git.js 100.00% <100.00%> (ø)
packages/dd-trace/src/plugins/util/test.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9baa26d...70b030a. Read the comment docs.

@juan-fernandez juan-fernandez merged commit 8fc437e into master Mar 25, 2021
@juan-fernandez juan-fernandez deleted the juan-fernandez/ci-app-git-metadata branch March 25, 2021 16:52
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