Skip to content

Conversation

@AlexITC
Copy link
Collaborator

@AlexITC AlexITC commented Apr 26, 2024

Follows up from #7410

@AlexITC AlexITC requested a review from a team as a code owner April 26, 2024 09:04

private val workflowOptions: WorkflowOptions = workflowDescriptor.workflowOptions

// TODO: Do we actually need this? it seems to be used only by tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if anyone can comment on this, this method seems unnecessary because its only used by the tests, the same occurs in PAPI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like completely dead code in Batch, fine to remove IMO. IntelliJ doesn't even find any references in tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I skipped porting these tests but I wanted to double check before removing the code.

Thanks for looking into it.


private val workflowOptions: WorkflowOptions = workflowDescriptor.workflowOptions

// TODO: Do we actually need this? it seems to be used only by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like completely dead code in Batch, fine to remove IMO. IntelliJ doesn't even find any references in tests.

@aednichols aednichols changed the title Port more tests from papi-common to the GCP Batch backend WX-1595 Port more tests from papi-common to the GCP Batch backend May 14, 2024
@aednichols aednichols enabled auto-merge (squash) May 15, 2024 14:00
@aednichols aednichols merged commit 5d14beb into develop May 15, 2024
@aednichols aednichols deleted the gcp-batch-add-tests branch May 15, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants