-
-
Notifications
You must be signed in to change notification settings - Fork 84
test: overhaul automated compatibility coverage #123
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
Changes from all commits
20eb72d
c62ee67
e383455
90ae2f9
264b7bb
bd5d73f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,19 +18,20 @@ | |
|
|
||
| import com.ctrip.framework.apollo.config.data.extension.properties.ApolloClientProperties; | ||
| import com.ctrip.framework.apollo.core.spi.Ordered; | ||
| import java.util.function.Consumer; | ||
| import org.apache.commons.logging.Log; | ||
| import org.springframework.boot.context.properties.bind.BindHandler; | ||
| import org.springframework.boot.context.properties.bind.Binder; | ||
| import org.springframework.boot.web.reactive.function.client.WebClientCustomizer; | ||
| import org.springframework.lang.Nullable; | ||
| import org.springframework.web.reactive.function.client.WebClient; | ||
|
|
||
| /** | ||
| * @author vdisk <vdisk@foxmail.com> | ||
| */ | ||
| public interface ApolloClientWebClientCustomizerFactory extends Ordered { | ||
|
||
|
|
||
| /** | ||
| * create a WebClientCustomizer instance | ||
| * create a webclient builder customizer | ||
| * | ||
| * @param apolloClientProperties apollo client binded properties | ||
| * @param binder properties binder | ||
|
|
@@ -41,10 +42,10 @@ public interface ApolloClientWebClientCustomizerFactory extends Ordered { | |
| * Spring Boot 3.x or | ||
| * org.springframework.boot.bootstrap.ConfigurableBootstrapContext | ||
| * for Spring Boot 4.x) | ||
| * @return WebClientCustomizer instance or null | ||
| * @return customizer instance or null | ||
| */ | ||
| @Nullable | ||
| WebClientCustomizer createWebClientCustomizer(ApolloClientProperties apolloClientProperties, | ||
| Consumer<WebClient.Builder> createWebClientCustomizer(ApolloClientProperties apolloClientProperties, | ||
| Binder binder, BindHandler bindHandler, Log log, | ||
| Object bootstrapContext); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
🧩 Analysis chain
🌐 Web query:
codecov/codecov-action v4 token requirement documentation💡 Result:
For
codecov/codecov-action@v4, tokenless uploads are not supported (i.e., you should provideCODECOV_TOKENviawith: token:orenv: CODECOV_TOKEN). The one exception called out in the v4 docs is that PRs from forks into an upstream public repo can upload without the upstream repo’s token. [1]Codecov’s token docs also explain that private repos always require a token, and that tokenless uploading for public repos depends on org/repo settings—but the docs note that disabling token authentication for public repos is only available on
codecov-action> v5.0 (orcodecov-cli> 0.9), reinforcing that v4 generally requires tokens. [2]Primary references: Codecov Action README (v4 release notes) [1], Codecov “Codecov Tokens” documentation [2].
Sources: [1] (github.com) [2] (docs.codecov.com)
Citations:
codecov-action@v4requires atoken— upload will fail without it.Starting with v4,
codecov/codecov-actionrequiresCODECOV_TOKENto be explicitly provided. Without it, the coverage upload step will fail in most scenarios. The only exception is pull requests from forks into public upstream repositories.Proposed fix
- name: Upload coverage to Codecov uses: codecov/codecov-action@v4 with: files: ${{ github.workspace }}/apollo-*/target/site/jacoco/jacoco.xml + token: ${{ secrets.CODECOV_TOKEN }}📝 Committable suggestion
🤖 Prompt for AI Agents
Uh oh!
There was an error while loading. Please reload this page.
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.
Thanks for the note. I’m intentionally not forcing a token here: this repo runs as a public upstream + fork PR flow, and
codecov-action@v4is working in this setup (coverage upload passed on the latest run). Adding a required secret here may cause fork-PR visibility issues. If uploads become unstable later, I’ll follow up with a dedicated token/action upgrade change.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.
@nobodyiamThank you for the clarification! Your approach makes perfect sense — since the repository operates as a public upstream with fork PRs, you're correctly leveraging the exception wherecodecov-action@v4uploads without a token. The successful coverage uploads in this PR confirm that setup is working as expected, and avoiding secret visibility concerns for fork contributors is a good practice.I appreciate you explaining the context, and your plan to revisit if needed is reasonable. 👍
🧠 Learnings used