search

uutils / findutils

Rust implementation of findutils
MIT License
280 stars 35 forks source link

Provide GNU test comparison comments for PRs in Github Actions. #400

Closed hanbings closed 1 week ago

hanbings commented 4 weeks ago

link: https://github.com/uutils/findutils/issues/360

Add the PR comment code to compat.yml. But I don't have a good way to test this code yet, so I'm setting it to Daft for now.

codecov[bot] commented 4 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 60.48%. Comparing base (84e4be8) to head (1cd319d).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #400 +/- ## ======================================= Coverage 60.48% 60.48% ======================================= Files 32 32 Lines 4069 4069 Branches 895 895 ======================================= Hits 2461 2461 Misses 1254 1254 Partials 354 354 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sylvestre commented 3 weeks ago

nice ! could you please fix the conflict? thanks

sylvestre commented 3 weeks ago

Testing is hard, don't hesitate if you need to land it to test it further

hanbings commented 2 weeks ago

Testing is hard, don't hesitate if you need to land it to test it further

It seems that the compatibility information in Annotations is extracted correctly, but Actions gets a 403 unauthorized error when posting a comment. I guess the Token is not valid. Do we have a Secret named GITHUB_TOKEN or another Secret with comment permissions set in the repo?

and... I'm sorry for spending so much time testing.

sylvestre commented 2 weeks ago

@tertsdiepraam do you remember if it can be tested inside a PR or it needs to be merged first ?

tertsdiepraam commented 2 weeks ago

I think it had to be merged...

tertsdiepraam commented 2 weeks ago

Also, I split it into 2 workflows on purpose, so that the workflow that posts the comments could have more permissions but the other workflow could be run from the PR. We should probably do the same here. See also some of the discussions and stuff around this feature in coreutils: https://github.com/uutils/coreutils/pull/4010

hanbings commented 1 week ago

I'm separating the code for sending PR comments into a new CI file, but Github Actions only runs the CI file that exists in the default branch. I'm not sure the code will work, can we merge this PR first and then fix the code issues?

sylvestre commented 1 week ago

sure, i guess it isn't a draft anymore then ?

sylvestre commented 1 week ago

done

sylvestre commented 1 week ago

it is marked as skipped ?!

https://github.com/uutils/findutils/actions/workflows/comment.yml

hanbings commented 1 week ago

yeah, I feel weird about that. I'll fix it in my fork. Thanks.

sylvestre commented 1 week ago

yeah, I feel weird about that. I'll fix it in my fork. Thanks.

don't hesitate to copy what is done in the coreutils :)