-
Notifications
You must be signed in to change notification settings - Fork 299
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
[#1488]: fix(CI): make jacoco report work properly #2747
base: main
Are you sure you want to change the base?
Conversation
Hi @yuqi1129 , I'm thinking that this issue may not be resolved in repository level. |
Update: It's the restriction on pull requests.
Solutions from docs:
|
@yijhenlin Would you please guide as on how to do this? I think it worked before, not sure which change makes it fail to work. |
@jerryshao Currently, the recommended way is through Github App to generate an authentication token with fine-grained permission and use this token for public repository. |
Thanks for the explanation, let me check this. |
Hi @jerryshao |
- name: Jacoco Report to PR | ||
id: jacoco | ||
uses: madrapps/jacoco-report@v1.6.1 | ||
uses: yijhenlin/jacoco-report@summary_mode |
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.
So you build your own github action?
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.
hi @yijhenlin Thank you for your contribution.
You fixed jacoco issue in your github repo? yijhenlin/jacoco-report@4305a5a
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.
Yes, I fork from madrapps/jacoco-report and patch an option to check if this idea works or not, since it didn't expose the report's raw data to user level.
I'll try to contribute back to madrapps/jacoco-report if the workaround is acceptable and you can fork your own too.
@xunliu what's your opinion on it? |
I'm fork yijhenlin/jacoco-report@4305a5a to Datastrato org? |
What changes were proposed in this pull request?
-PskipITs
is passed3. Add pull-requests write permissions in workflow(not work for forked repo)Why are the changes needed?
JacocoReport
does not work #1488 (comment)catalogs/catalog-jdbc-postgresql
,clients/client-java
, etc.Error: HttpError: Resource not accessible by integration
Fix: #1488
Does this PR introduce any user-facing change?
N/A
How was this patch tested?
N/A