Skip to content

Conversation

teetangh
Copy link
Contributor

@teetangh teetangh commented Mar 24, 2025

No description provided.

…c-imports in GitHub Actions and update related configurations in check_diff.py.
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Hello @teetangh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses invalid workflows by commenting out references to test-doc-imports and test-pydantic in .github/scripts/check_diff.py and .github/workflows/check_diffs.yml. These workflows appear to be missing or invalid, and the changes prevent errors related to these missing workflows.

Highlights

  • Workflow Fix: Comments out references to the test-pydantic workflow in .github/scripts/check_diff.py to prevent errors due to a missing workflow definition. Specifically, the conditional logic that executes the pydantic tests is disabled.
  • Workflow Update: Removes test-pydantic from the list of jobs that require dependents in .github/scripts/check_diff.py.
  • Workflow Configuration: Comments out references to the test-doc-imports workflow in .github/scripts/check_diff.py to prevent errors due to a missing workflow definition. The conditional logic that executes the doc imports tests is disabled.
  • Workflow Definition: Comments out the definitions for the test-pydantic and test-doc-imports jobs in .github/workflows/check_diffs.yml.
  • CI Success Condition: Removes test-doc-imports and test-pydantic from the list of needs for the ci_success job in .github/workflows/check_diffs.yml.

Changelog

  • .github/scripts/check_diff.py
    • Comments out the test-pydantic workflow reference in _get_configs_for_single_dir function (lines 116-118).
    • Removes test-pydantic from the list of jobs in the elif condition on line 217.
    • Comments out the test-pydantic entry in map_job_to_configs dictionary (line 325).
    • Comments out the test-doc-imports entry in map_job_to_configs dictionary (lines 328-331).
  • .github/workflows/check_diffs.yml
    • Comments out test-doc-imports and test-pydantic from the outputs list in the build job (lines 43-44).
    • Comments out the entire test-pydantic job definition (lines 73-86).
    • Comments out the entire test-doc-imports job definition (lines 88-99).
    • Removes test-doc-imports and test-pydantic from the needs list for the ci_success job (line 162).
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Trivia time!

What is YAML, the language used to define GitHub Actions workflows, short for?

Click here for the answer
YAML stands for "YAML Ain't Markup Language". It's a recursive acronym, emphasizing that YAML is a data serialization language, not a markup language.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to fix invalid workflows by commenting out references to the test-pydantic and test-doc-imports workflows in the .github/scripts/check_diff.py and .github/workflows/check_diffs.yml files. The changes seem reasonable given the context of missing workflows.

Merge Readiness

The changes in this pull request address the issue of invalid workflows by commenting out the references to the missing workflows. Given that there are no critical or high severity issues, the pull request appears to be ready for merging. However, I am unable to directly approve the pull request, and it is recommended that other reviewers examine the changes before merging.

Comment on lines +116 to +118
# Commenting out missing workflow reference
# if job == "test-pydantic":
# return _get_pydantic_test_configs(dir_)

Choose a reason for hiding this comment

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

medium

Consider adding a comment explaining why these workflows are being commented out, and if there are plans to re-enable them in the future. This will help future developers understand the context of these changes.

Comment on lines 217 to +218
)
elif job in ["test", "compile-integration-tests", "dependencies", "test-pydantic"]:
elif job in ["test", "compile-integration-tests", "dependencies"]:

Choose a reason for hiding this comment

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

medium

It's good to remove the test-pydantic job from this list since it's no longer a valid job. Ensure that this change aligns with the intended behavior of the script.

Comment on lines +328 to +331
# Commenting out missing workflow
# map_job_to_configs["test-doc-imports"] = (
# [{"python-version": "3.12"}] if docs_edited else []
# )

Choose a reason for hiding this comment

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

medium

Similar to the previous comment, adding a brief explanation for commenting out this section would be beneficial for future maintainers.

Comment on lines +43 to +44
# test-doc-imports: ${{ steps.set-matrix.outputs.test-doc-imports }}
# test-pydantic: ${{ steps.set-matrix.outputs.test-pydantic }}

Choose a reason for hiding this comment

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

medium

Adding a comment explaining why these outputs are being commented out would be helpful.

Comment on lines +73 to +86
# Commenting out missing workflow reference
# test-pydantic:
# name: cd ${{ matrix.job-configs.working-directory }}
# needs: [ build ]
# if: ${{ needs.build.outputs.test-pydantic != '[]' }}
# strategy:
# matrix:
# job-configs: ${{ fromJson(needs.build.outputs.test-pydantic) }}
# fail-fast: false
# uses: ./.github/workflows/_test_pydantic.yml
# with:
# working-directory: ${{ matrix.job-configs.working-directory }}
# pydantic-version: ${{ matrix.job-configs.pydantic-version }}
# secrets: inherit

Choose a reason for hiding this comment

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

medium

Adding a comment explaining why this job is being commented out would be helpful.

Comment on lines +88 to +99
# Commenting out missing workflow reference
# test-doc-imports:
# needs: [ build ]
# if: ${{ needs.build.outputs.test-doc-imports != '[]' }}
# strategy:
# matrix:
# job-configs: ${{ fromJson(needs.build.outputs.test-doc-imports) }}
# fail-fast: false
# uses: ./.github/workflows/_test_doc_imports.yml
# secrets: inherit
# with:
# python-version: ${{ matrix.job-configs.python-version }}

Choose a reason for hiding this comment

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

medium

Adding a comment explaining why this job is being commented out would be helpful.

@@ -159,7 +161,7 @@
echo "$STATUS" | grep 'nothing to commit, working tree clean'
ci_success:

Choose a reason for hiding this comment

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

medium

Removing test-doc-imports and test-pydantic from the needs list is consistent with the removal of those jobs. This ensures that the ci_success job doesn't wait for jobs that no longer exist.

@teetangh teetangh changed the title (fix) invalid workflows -2 (fix) invalid workflows by commenting Mar 24, 2025
@teetangh teetangh added bug Something isn't working help wanted Extra attention is needed labels Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant