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

Allow post the report to job summary #68

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

06393993
Copy link

@06393993 06393993 commented Jul 29, 2023

For PR from a foreign repo, the action won't have enough permission to comment on a PR, and for security reason the action shouldn't have the permission. See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/, https://github.com/orgs/community/discussions/26644.

This PR implements the feature requested in #65.

I haven't tested this PR yet. I may need to upgrade the version of @actions/core to call the new summary API.

EDIT: I have tested this PR, and people can check the output here. The job summary also won't generate new email, so may also solve #57.

@06393993
Copy link
Author

06393993 commented Jul 5, 2024

Hi @romeovs , can you please take a look at this PR? Thanks a lot!

@daxpedda
Copy link

I've tried using the latest commit from your fork but it errors out with:

file:///home/runner/work/_actions/06393993/lcov-reporter-action/c2dc6744871841e7cc2fa528f983253ecee694ab/dist/main.js:3
var require$$0$1 = require('node:fs');
                   ^

ReferenceError: require is not defined in ES module scope, you can use import instead
This file is being treated as an ES module because it has a '.js' file extension and '/home/runner/work/_actions/06393993/lcov-reporter-action/c2dc6744871841e7cc2fa528f983253ecee694ab/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at file:///home/runner/work/_actions/06393993/lcov-reporter-action/c2dc6744871841e7cc2fa528f983253ecee694ab/dist/main.js:3:20
    at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5)

Node.js v20.13.1

Using an older commit I took from vk-layer-for-rust works.

@06393993
Copy link
Author

Re #68 (comment), I had a simple fix to change dist/main.js to dist/main.cjs. But it seems that 423a24f breaks the script completely, so I need to adjust more code to fix that issue.

* Upgrade yarn to 1.22.22, as asdf fails to install the 1.22.21 version due to 404.
* Change how the github client is created according to the latest API of @actions/github.
* Upgrade rollup versions, including rollup, rollup-plugin-node-externals, @rollup/plugin-commonjs, @rollup/plugin-node-resolve
* Change the node package type to module, and rename the rollup.config.js to *.mjs
* Change the geneartion file to dist/main.cjs
* Regenerate dist/main.cjs via yarn run build
... which adds support to job summary.
@06393993 06393993 force-pushed the master branch 2 times, most recently from ee6b8c6 to 9ca8f32 Compare August 15, 2024 21:13
@06393993
Copy link
Author

I've tried using the latest commit from your fork but it errors out with:

file:///home/runner/work/_actions/06393993/lcov-reporter-action/c2dc6744871841e7cc2fa528f983253ecee694ab/dist/main.js:3
var require$$0$1 = require('node:fs');
                   ^

ReferenceError: require is not defined in ES module scope, you can use import instead
This file is being treated as an ES module because it has a '.js' file extension and '/home/runner/work/_actions/06393993/lcov-reporter-action/c2dc6744871841e7cc2fa528f983253ecee694ab/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at file:///home/runner/work/_actions/06393993/lcov-reporter-action/c2dc6744871841e7cc2fa528f983253ecee694ab/dist/main.js:3:20
    at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5)

Node.js v20.13.1

Using an older commit I took from vk-layer-for-rust works.

I have updated this PR and verified that it works.

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