Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 9, 2025

Creates dedicated CI/CD workflows for the bnf/ package, mirroring the root-level pytest.yml and jest.yml structure but scoped to the BNF subdirectory.

Key changes:

  • .github/workflows/bnf-pytest.yml: Python testing (3.10-3.14) and PyPI publishing

    • All steps run in working-directory: bnf
    • Uses StoneMoe/setup-antlr4@v4 to install ANTLR4 for parser generation
    • Builds pure Python package with python -m build (universal wheel, no cross-platform builds needed)
    • Publishes apyds-bnf to PyPI using pypi-bnf environment
  • .github/workflows/bnf-jest.yml: JavaScript testing (Node 20/22/24) and npm publishing

    • All steps run in working-directory: bnf
    • Uses StoneMoe/setup-antlr4@v4 to install ANTLR4 for parser generation
    • Configures cache-dependency-path: bnf/package-lock.json for npm caching
    • Removes emsdk setup (not needed for BNF package)
    • Publishes atsds-bnf to npm using npm-bnf environment

Both workflows include standard test jobs triggered on PR/push, plus conditional publish jobs on tag pushes.

Original prompt

This section details on the original issue you should resolve

<issue_title>模仿 .github/workflows/pytest.yml 和 .github/workflows/jest.yml 写一个适用于 bnf下面的俩workflow</issue_title>
<issue_description>名字就叫 bnf-pytest.yml 和 bnf-jest.yml 吧</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Add bnf-pytest.yml and bnf-jest.yml workflows Add bnf-pytest.yml and bnf-jest.yml workflows with dedicated publish environments Dec 9, 2025
Copilot AI requested a review from hzhangxyz December 9, 2025 04:03
@hzhangxyz hzhangxyz marked this pull request as ready for review December 9, 2025 05:04
Copilot AI review requested due to automatic review settings December 9, 2025 05:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR creates dedicated GitHub Actions workflows for the bnf/ subdirectory package, enabling independent testing and publishing for the apyds-bnf (Python) and atsds-bnf (JavaScript) packages that provide BNF parsing functionality.

Key changes:

  • Adds Python testing and PyPI publishing workflow for the pure Python BNF package
  • Adds JavaScript testing and npm publishing workflow for the BNF TypeScript/JavaScript package
  • Configures dedicated publishing environments (pypi-bnf and npm-bnf) to separate BNF package releases from root package releases

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/bnf-pytest.yml Python CI/CD workflow for BNF package testing (pytest) and PyPI publishing with cross-platform wheel building
.github/workflows/bnf-jest.yml JavaScript CI/CD workflow for BNF package testing (jest) and npm publishing, with emsdk setup removed as it's not needed for this package

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 37 to 69
wheels:
runs-on: ${{ matrix.os }}
needs: bnf-pytest

if: "github.event_name == 'push' && startsWith(github.ref, 'refs/tags')"

strategy:
fail-fast: false
matrix:
os: [windows-latest, ubuntu-latest, macos-latest]

steps:
- uses: actions/checkout@v6

- name: recovery tag information
run: git fetch --tags --force

- uses: actions/setup-python@v6
with:
python-version: ${{ env.PYTHON_LATEST_VERSION }}
cache: 'pip'

- name: install cibuildwheel
run: pip install cibuildwheel

- name: build wheels
working-directory: bnf
run: python -m cibuildwheel

- uses: actions/upload-artifact@v5
with:
name: wheels-${{ matrix.os }}
path: bnf/wheelhouse/*.whl
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The apyds-bnf package is a pure Python package (using setuptools.build_meta backend with no C extensions), but this workflow uses cibuildwheel to create platform-specific wheels across Windows, Ubuntu, and macOS. This is unnecessary and wasteful of CI resources.

For pure Python packages, you should build a universal wheel using standard build tools instead. Consider replacing the wheels job with a simpler approach:

build:
  runs-on: ubuntu-latest
  needs: bnf-pytest
  
  if: "github.event_name == 'push' && startsWith(github.ref, 'refs/tags')"
  
  steps:
    - uses: actions/checkout@v6
    
    - name: recovery tag information
      run: git fetch --tags --force
    
    - uses: actions/setup-python@v6
      with:
        python-version: ${{ env.PYTHON_LATEST_VERSION }}
    
    - name: build
      working-directory: bnf
      run: |
        pip install build
        python -m build
    
    - uses: actions/upload-artifact@v5
      with:
        name: dist
        path: bnf/dist/*

Then update the upload job to use name: dist instead of pattern: wheels-*.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced the wheels job with a simpler build job using python -m build for the pure Python package. Also added ANTLR4 setup as required. (bc8622c)

Comment on lines 84 to 86
pattern: wheels-*
path: dist
merge-multiple: true
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The upload job expects artifacts with pattern wheels-*, but since this should be changed to build a universal wheel instead of platform-specific wheels (see previous comment), this artifact download configuration needs to be updated.

If the wheels job is replaced with a simple build job that produces a single artifact named dist, update this to:

- uses: actions/download-artifact@v6
  with:
    name: dist
    path: dist
Suggested change
pattern: wheels-*
path: dist
merge-multiple: true
name: dist
path: dist

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to download single dist artifact instead of wheels-* pattern. (bc8622c)

Copy link
Member

@hzhangxyz hzhangxyz left a comment

Choose a reason for hiding this comment

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

  1. 你需要安装antlr4在系统中, 才能进行python或js的build. 你可以使用 StoneMoe/setup-antlr4@v4 这个 action完成
  2. apyds_bnf这个不需要cibuildwheel, 因为是纯python的, 直接python -m build 就可以了

Copy link
Contributor Author

Copilot AI commented Dec 9, 2025

  1. 你需要安装antlr4在系统中, 才能进行python或js的build. 你可以使用 StoneMoe/setup-antlr4@v4 这个 action完成
  2. apyds_bnf这个不需要cibuildwheel, 因为是纯python的, 直�...

Added StoneMoe/setup-antlr4@v4 action to both workflows for ANTLR4 installation, and replaced cibuildwheel with python -m build for the pure Python package. Also fixed cache-dependency-path for npm. (bc8622c)

@hzhangxyz hzhangxyz force-pushed the copilot/add-bnf-pytest-and-jest-workflows branch 4 times, most recently from a670ed4 to fdf56e1 Compare December 9, 2025 08:07
@hzhangxyz hzhangxyz merged commit 53b4bb8 into main Dec 9, 2025
73 checks passed
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.

模仿 .github/workflows/pytest.yml 和 .github/workflows/jest.yml 写一个适用于 bnf下面的俩workflow

2 participants