-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Add in better debugging logs #132
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files
☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will decrease total bundle size by 992.73kB ⬇️
|
Bundle ReportChanges will decrease total bundle size by 24.6kB ⬇️
|
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.
What is the purpose of all these provider files?
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.
They detect environment variables present in different CI providers so we can collect them and send them to the API such as branch, commit sha, etc.
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 haven't configured BA myself yet (probably ought to give it a go though), but this is really cool, but also seems like a PITA to maintain lol. Is there testing and/or monitoring in place to detect breaking changes by CI providers? Why have these providers been selected? I hadn't heard of half of these before lol. Are they actually users on each of these?
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.
You see ... Codecov already wrote (almost) all of these utilities in the Node uploader ... so most of them I myself didn't write. Yea it's understandable that there's lots, however I don't think many companies would do well changing CI env vars without a long run time notification ahead of time since many systems could break if they changed them
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.
Looks fine! I think I understand more or less what you've done here. Left a couple of curiosity questions. Missing coverage on a few lines too.
Description
This PR adds back in debug logs for to inform the user of what commit sha has been chosen, as well as some extra logs around which commit sha is being chosen in cases like GH merge commits, so we can debug some issues around the wrong commit being selected.
Closes codecov/engineering-team#1751
Notable Changes
debug
log util to be enabled/disabled depending on passed in debug setting