chore(codecs): replace native encoding fixture patch files with cfg-gated code#24971
chore(codecs): replace native encoding fixture patch files with cfg-gated code#24971thomasqueirozb wants to merge 9 commits intomasterfrom
Conversation
github-actions
bot
added
the
domain: core
thomasqueirozb
added
the
no-changelog
thomasqueirozb
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8eaf3cc3b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vector-vrl-category = { path = "lib/vector-vrl/category" } | ||
| vector-vrl-functions = { path = "lib/vector-vrl/functions", default-features = false } | ||
| vrl = { git = "https://github.com/vectordotdev/vrl.git", branch = "main", default-features = false, features = ["arbitrary", "cli", "test", "test_framework", "stdlib-base"] } | ||
| vrl = { git = "https://github.com/vectordotdev/vrl.git", branch = "generate-fixtures", default-features = false, features = ["arbitrary", "cli", "test", "test_framework", "stdlib-base"] } |
There was a problem hiding this comment.
Switching the workspace vrl dependency from main to the topic branch generate-fixtures makes every normal Vector build depend on an integration branch that is only needed for fixture generation. Because this is declared in the shared workspace dependency, all packages pull from that branch even when generate-fixtures is not enabled; if the branch is force-pushed or removed, dependency resolution breaks for CI and developer builds. Please keep the default dependency on a stable upstream ref (or pin an immutable rev) and scope fixture-only wiring behind the feature path instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex @thomasqueirozb will revert the above to main but after this vectordotdev/vrl#1711 is merged first. So you can ignore this finding.
There was a problem hiding this comment.
Understood — thanks for the clarification.
Given your note that this will be reverted to main after vectordotdev/vrl#1711 merges, I’m treating that review finding on Cargo.toml (line 220) as resolved/no action needed for this PR and will not make any follow-up code changes for it.
2 participants
Summary
Replaces the loose patch files (
vector_generate_fixtures.patch,vrl_generate_fixtures.patch) used to regenerate native encoding test fixtures with propergenerate-fixturesfeature flags in both this repo and the VRL repo, plus a standalone binary that writes fixtures directly to their committed location.Key changes:
generate-fixturesfeature invector-coreactivates fixture-stableArbitraryimpls (NaN-safe f64, non-empty names, no timestamps) and pulls invrl/generate-fixturesautomaticallyArbitraryimpls moved fromtest/common.rstoevent/arbitrary_impl.rs, compiled underany(test, feature = "generate-fixtures")u64::arbitrary(g)instead ofGen::new()(which calledfrom_entropy()) to ensure full determinismAgentDDSketch::set_sum_avgmethod gated on the feature instead of making fieldspubcargo run -p vector-core --features generate-fixtures --bin generate-fixtures) writes directly tolib/codecs/tests/data/native_encoding/Vector configuration
NA
How did you test this PR?
Ran the binary twice and confirmed identical checksums. Ran
cargo test -p vector-coreto confirm existing tests pass.Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References