Skip to content

Conversation

@hanouticelina
Copy link
Contributor

@hanouticelina hanouticelina commented Mar 3, 2025

Following @sayakpaul's work introducing a GitHub style bot in diffusers huggingface/diffusers#10274 and this slack conversation (private), this PR moves the workflow into a GitHub Action making it a reusable workflow across other repos. For example in diffusers, the workflow will become:

name: Style Bot

on:
  issue_comment:
    types: [created]

permissions:
  contents: write
  pull-requests: write

jobs:
  style:
    uses: huggingface/huggingface_hub/.github/workflows/style-bot-action.yml@main
    with:
      python_quality_dependencies: "[quality]"
      style_command: "make style && make quality"
      pre_commit_script: |
        curl -o main_Makefile https://raw.githubusercontent.com/huggingface/diffusers/main/Makefile
        curl -o main_setup.py https://raw.githubusercontent.com/huggingface/diffusers/refs/heads/main/setup.py
        curl -o main_check_doc_toc.py https://raw.githubusercontent.com/huggingface/diffusers/refs/heads/main/utils/check_doc_toc.py
        
        diff_failed=0
        if ! diff -q main_Makefile Makefile; then
          echo "Error: The Makefile has changed. Please ensure it matches the main branch."
          diff_failed=1
        fi
        if ! diff -q main_setup.py setup.py; then
          echo "Error: The setup.py has changed. Please ensure it matches the main branch."
          diff_failed=1
        fi
        if ! diff -q main_check_doc_toc.py utils/check_doc_toc.py; then
          echo "Error: The utils/check_doc_toc.py has changed. Please ensure it matches the main branch."
          diff_failed=1
        fi
        if [ $diff_failed -eq 1 ]; then
          echo "❌ Error happened as we detected changes in the files that should not be changed ❌"
          exit 1
        fi
        echo "No changes in the files. Proceeding..."
        rm -rf main_Makefile main_setup.py main_check_doc_toc.py

    secrets:
      github_token: ${{ secrets.GITHUB_TOKEN }}

Note: pre_commit_script input was added to be able to do some repo-specific steps before committing.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@hanouticelina hanouticelina marked this pull request as ready for review March 4, 2025 08:59
@hanouticelina
Copy link
Contributor Author

Tested the workflow here : https://github.com/hanouticelina/huggingface_hub/pull/1 and it works like a charm 😃 I tested also with a pre_commit_script similar to the one in diffusers, all good!

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Lovely!

@ydshieh
Copy link
Contributor

ydshieh commented Mar 4, 2025

Nice to have this reusable workflow.

Let's try to avoid any ${{ ... }} within run command - for security reasons.

(some of them are OK probably, but if we can avoid using any of it, it's easier for review)

@hanouticelina
Copy link
Contributor Author

hanouticelina commented Mar 4, 2025

Let's try to avoid any ${{ ... }} within run command - for security reasons.

@ydshieh do you have an alternative to substitute a command?

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thank you! Looking forward to using it on a practical use case 😄
Regarding the run command I do think it's important to have it configurable -if there is any way to make it secure enough-.

Copy link
Contributor

Choose a reason for hiding this comment

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

love it!

@ydshieh
Copy link
Contributor

ydshieh commented Mar 4, 2025

Let's try to avoid any ${{ ... }} within run command - for security reasons.

@ydshieh do you have an alternative to substitute a command?

For example

      - name: Install dependencies
        run: |
          python -m pip install --upgrade pip
          pip install .${{ inputs.python_quality_dependencies }}

you can make it

      - name: Install dependencies
        env: python_quality_dependencies
          python_quality_dependencies: ${{ inputs.python_quality_dependencies }}
        run: |
          python -m pip install --upgrade pip
          pip install .$python_quality_dependencies

You can see this is applied in many places.

Note this is necessary for security for any context that could be related to user inputs like branch name. For inputs.pre_commit_script, it's unlikely being changed, especially the workflow file is trigger with issue_comment that is only happening if the workflow file in on the main branch. So it is not really a necessary change in your work here. But we never know if a (new) workflow caller is going to be triggered in another kind of events.

@hanouticelina
Copy link
Contributor Author

thanks a lot @ydshieh for the suggestion! I addressed that in 04306f3 and it works correctly.

@hanouticelina
Copy link
Contributor Author

merging!

@hanouticelina hanouticelina merged commit edcc774 into main Mar 4, 2025
19 checks passed
@hanouticelina hanouticelina deleted the style-bot-gh-action branch March 4, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants