Skip to content
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.

Sign up for GitHub

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

Jump to bottom

convert spring batch tests to java #12004

Merged
merged 17 commits into from
Sep 13, 2024
Merged

convert spring batch tests to java #12004

merged 17 commits into from
Sep 13, 2024

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Aug 13, 2024 •
edited by jaydeluca
Loading

Related to #7195

@zeitlinger zeitlinger self-assigned this Aug 13, 2024
@zeitlinger zeitlinger requested a review from a team August 13, 2024 09:12
@zeitlinger zeitlinger mentioned this pull request Aug 14, 2024
Open
@jaydeluca
Copy link
Member

would it be possible to break this into a couple smaller PRs for easier review?

@zeitlinger
Copy link
Member Author

would it be possible to break this into a couple smaller PRs for easier review?

all tests use the JobRunner interface to work
I could put that in a separate PR, but I'm not sure it makes it easier to review

@zeitlinger zeitlinger force-pushed the spring-batch-tests-to-java branch from 7667682 to 51761cf Compare August 19, 2024 05:43
@trask
Copy link
Member

trask commented Aug 19, 2024

would it be possible to break this into a couple smaller PRs for easier review?

all tests use the JobRunner interface to work I could put that in a separate PR, but I'm not sure it makes it easier to review

not sure I follow why you can't split this into smaller PRs? they can be stacked if needed

@zeitlinger zeitlinger force-pushed the spring-batch-tests-to-java branch from 51761cf to 1ee063a Compare August 21, 2024 15:50
@zeitlinger zeitlinger mentioned this pull request Aug 21, 2024
Merged
@zeitlinger
Copy link
Member Author

not sure I follow why you can't split this into smaller PRs? they can be stacked if needed

ok I gave it a shot at #12076

@zeitlinger zeitlinger marked this pull request as draft August 22, 2024 07:07
@zeitlinger zeitlinger force-pushed the spring-batch-tests-to-java branch from 2174b7e to 689b2e5 Compare August 29, 2024 11:18
@zeitlinger
Copy link
Member Author

would it be possible to break this into a couple smaller PRs for easier review?

all tests use the JobRunner interface to work I could put that in a separate PR, but I'm not sure it makes it easier to review

not sure I follow why you can't split this into smaller PRs? they can be stacked if needed

with the first PR merged, this is the remaining changes - let me know if this is good as it is

@zeitlinger zeitlinger marked this pull request as ready for review August 29, 2024 11:40
@zeitlinger
Copy link
Member Author

@laurit please check again

zeitlinger and others added 5 commits September 10, 2024 12:57
@zeitlinger
convert spring batch tests to java
c6bbb67
@zeitlinger
fix tests
fa428ca
@zeitlinger
pr review
b767619
@zeitlinger @steverao
Update instrumentation/spring/spring-batch-3.0/javaagent/src/test/jav…
e8cc48f 
…a/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/event/JsrConfigCustomSpanEventTest.java

Co-authored-by: Steve Rao <[email protected]>
@zeitlinger
format
0b7c334
@zeitlinger zeitlinger force-pushed the spring-batch-tests-to-java branch from fa5b22e to 53c7b96 Compare September 10, 2024 10:57
@zeitlinger
Copy link
Member Author

@laurit please check again

@trask trask merged commit e36be9c into open-telemetry:main Sep 13, 2024
56 checks passed
@zeitlinger zeitlinger deleted the spring-batch-tests-to-java branch September 13, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steverao steverao steverao left review comments

@trask trask trask left review comments

@laurit laurit laurit approved these changes

Assignees

@zeitlinger zeitlinger

Labels
area:tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zeitlinger @jaydeluca @trask @laurit @steverao