Skip to content

Conversation

@poojantcs
Copy link
Contributor

@poojantcs poojantcs commented Apr 17, 2023

@itsankit-google
Copy link
Contributor

I suggested to add @Oracle_Required tag so that we can verify the tests run successfully with github actions.

But before merging the PR, please check with @Aryan-Verma and @MrRahulSharma if we need to add the tag to all or some specific tests because overall the required tests should not take more than 15mins to avoid blocking PR merges for longer time.

@Vipinofficial11 Vipinofficial11 force-pushed the e2e_oracle branch 2 times, most recently from 5af57ea to 3cb7c79 Compare May 19, 2023 15:34
Copy link
Contributor

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is an issue currently with the oracle tests running indefinitely.

Please fix before merging the PR.

@bharatgulati bharatgulati force-pushed the e2e_oracle branch 3 times, most recently from 059b3e9 to 3ea051a Compare June 7, 2023 12:36
@bharatgulati bharatgulati force-pushed the e2e_oracle branch 3 times, most recently from 5a98b49 to 87671f7 Compare June 14, 2023 12:23
@itsmekumari itsmekumari force-pushed the e2e_oracle branch 4 times, most recently from 24b710c to 9155e2f Compare July 14, 2023 08:49
@poojantcs
Copy link
Contributor Author

Details

Hi @itsankit-google , we have a green build now and the commits are squashed as well.

@MrRahulSharma : Can we please merge this PR?

* BQValidation.
*/

public class BQValidation {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this class not present in the same top level package as OracleClient?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such reason to use this class inside this package. Do we need to add it to OracleClient package, as we are using a different package for BQValidation class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep all the common classes at one place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change done.

@itsankit-google
Copy link
Contributor

Details

Hi @itsankit-google , we have a green build now and the commits are squashed as well.

@MrRahulSharma : Can we please merge this PR?

Sorry, didn't saw it, added one comment/question.

@itsankit-google itsankit-google merged commit 001496a into data-integrations:develop Jul 20, 2023
@psainics psainics deleted the e2e_oracle branch January 22, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants