-
Notifications
You must be signed in to change notification settings - Fork 218
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
Add logging to project automations #3404
Conversation
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.
@Proibito04 from a cursory reading, it seems great work, thank you for the contribution! I do have a few notes.
- The Lint check is failing, which indicates that your code diverges from the style guide. You can examine the output of the Lint check to determine what needs to be fixed.
- Debug logs are not visible in the GitHub UI unless the action is being re-run with debug level logging. Thus you should make some progress tracking logs higher level than debug so that they always show up.
- Since we are running these workflows inside
actions/github-script
, we can use the@actions/core
package (can be passed as an arg to themain
function) to write logs that will automatically handle the relevant::_::
prefix. - Some of the logs that are often repeated adjacent to a function or util can be moved into that function or util class itself. That will significant DRY up the code changes.
@dhruvkb, thank you for your suggestions. I've removed the imports and passed |
…/openverse into issue_3396
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.
This is a huge improvement, and thanks for constantly iterating on feedback! I want to emphasise that this is incredible as it is and we're only iterating on nit picks and minor implementation details now.
But as always, there is scope for more refinement. I've listed down some ideas as comments below. I hope you don't mind more iterations but if you do, please let us know and we'll be happy to apply the finishing touches and merge it.
I also made some changes to your code as an example with explanations below each change. If you could apply them in other areas, that would be awesome.
- Implemented core in Project class. - Removed core from moveCard functions. - Added core integration in YAML configurations.
Based on the low urgency of this PR, the following reviewers are being gently reminded to review this PR: @obulat Excluding weekend1 days, this PR was ready for review 5 day(s) ago. PRs labelled with low urgency are expected to be reviewed within 5 weekday(s)2. @Proibito04, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
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.
Thank you for your contribution, @Proibito04, great work!
console.log
statements for enhanced debugging in GitHu…
Fixes
Fixes #3396 by @dhruvkb
Description
This pull request adds detailed console.log statements for improved debugging in
issues.mjs
andprs.mjs
, enhancing visibility in GitHub Actions workflows.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin