Repository navigation
Migrate internal-api groovy files to java part 2 - #12766
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
we migrate tests: - ConfigProviderTest - OtelEnvironmentConfigSourceTest - StableConfigParserTest - BaggageTest - SpanLinkTest - TaskWrapperTest - URIDataAdapterTest - URIDefaultDataAdapterTest - URINoRawDataAdapterTest - URIRawDataAdapterTest - URIUtilsTest - UnparseableURIAdapterTest - Utf8ByteStringTest
6ac6cc6 to
12c67f3
Compare
bric3
left a comment
There was a problem hiding this comment.
Overall looks ok to me, I got a few suggestions, some I'd like to see in the PR before merging, some might be later improvements (non-blocking).
| "too few characters | '%' | '�' ", | ||
| "too few characters after one digit | '%1' | '�' ", | ||
| "illegal 1st character | '%W1' | '�' ", | ||
| "illegal 2nd character | '%1X' | '�' ", | ||
| "illegal 1st character in word | 'wh%Y1t' | 'wh�t' ", | ||
| "illegal 2nd character in word | 'wh%1Zt' | 'wh�t' ", | ||
| "too few characters at end | 'wh%' | 'wh�' ", | ||
| "too few characters after one digit at end | 'wh%1' | 'wh�' ", | ||
| "invalid 2 byte sequence | '%C3%28' | '�(' ", | ||
| "invalid sequence identifier | '%A0%A1' | '��' ", | ||
| "invalid 3 byte sequence in 2nd byte | '%E2%28%A1' | '�(�' ", | ||
| "invalid 3 byte sequence in 3rd byte | '%E2%82%28' | '�(' ", | ||
| "invalid 4 byte sequence in 2nd byte | '%F0%28%8C%BC' | '�(��' ", | ||
| "invalid 4 byte sequence in 3rd byte | '%F0%90%28%BC' | '�(�' ", | ||
| "invalid 4 byte sequence in 4th byte | '%F0%28%8C%28' | '�(�(' ", | ||
| "valid 5 byte sequence but not unicode | '%F8%A1%A1%A1%A1' | '�����' ", | ||
| "valid 6 byte sequence but not unicode | '%FC%A1%A1%A1%A1%A1' | '������' " |
There was a problem hiding this comment.
quibble (if-minor): I know it was there before, but I wonder if it could be safer to use escaped chars here.
There was a problem hiding this comment.
unresolving for visibility now that it's merged
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
we migrate tests:
Motivation
this is part of the effort to migrate groovy tests to Java/JUnit
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]