Skip to content

Conversation

@Valeriy-Burlaka
Copy link
Contributor

Description

Fixes #

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

@Valeriy-Burlaka Valeriy-Burlaka self-assigned this Dec 13, 2024
@Valeriy-Burlaka Valeriy-Burlaka requested review from a team as code owners December 13, 2024 11:18
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Dec 13, 2024
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will post a full review in a few minutes. In the meantime, here's a quick summary to get everyone up to speed.

This PR, "wip: trigger ci failure," appears to be a work in progress aimed at intentionally triggering a CI failure. The primary change is the addition of a single line comment # test at the end of generative_ai/embeddings/test_embeddings_examples.py. This suggests the author is likely testing the CI/CD pipeline's response to errors or is using this as a debugging step. The lack of substantial code changes beyond this comment indicates a focus on infrastructure or testing rather than feature development. Without more context, it's difficult to definitively state the full intent, but the name suggests a deliberate attempt to break the build process for testing purposes.

While the changes are minimal, they are crucial for ensuring the robustness of the CI/CD pipeline. Let's review the full impact once the author provides more details.

And now, a little haiku to lighten the mood:

Tests fail, code cries,
Debugging's endless journey,
Coffee keeps us warm.

@Valeriy-Burlaka Valeriy-Burlaka marked this pull request as draft December 13, 2024 11:25
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The PR title indicates work in progress and doesn't clearly communicate the purpose of the change. While triggering a CI failure might be the immediate goal, the title should reflect the underlying issue being tested or the functionality being developed. The description provides a standard checklist but lacks specific context about this particular change. It's important to provide clear and concise information in the PR description to help reviewers understand the purpose and scope of the changes. The added line in the test file seems intended to cause a CI failure, which is fine for testing purposes, but it should be removed or commented out once the actual functionality is implemented.

assert tuning_job._status.name != "PIPELINE_STATE_FAILED"
finally:
dispose(tuning_job)
# test
Copy link

Choose a reason for hiding this comment

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

This comment is a placeholder and doesn't provide any useful information about the test or the code. Since this is a test file, descriptive comments are crucial for explaining the purpose of each test case and the expected behavior. Consider adding details about what scenario this test covers, the expected outcome, and any relevant context. For example, you could explain which function or method is being tested, the specific inputs and expected outputs, and any edge cases or error conditions that are being handled. Once the actual code is in place, update the comment to reflect its functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant