CI step fails on comment change in setup.cfg

[ahonnecke]

Describe the bug

I'm attempting to open a PR from my fork, and it is failing on a no-op in the towncrier step.

https://github.com/beeware/toga-chart/pull/60

Steps to reproduce

1. Fork this repo
2. make a branch with a no-op change to setup.cfg
3. Push
4. Open PR from branch

Expected behavior

CI completes successfully.

Screenshots

Environment

• Operating System:
• Python version:
• Software versions:
  • Briefcase:
  • Toga:
  • ...

Logs

Run tox -e towncrier-check
towncrier-check: install_deps> python -I -m pip install 'towncrier~=22.8'
.pkg: install_requires> python -I -m pip install 'setuptools>=60' 'setuptools_scm[toml]>=7.0'
.pkg: _optional_hooks> python /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_sdist> python /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_wheel> python /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: install_requires_for_build_wheel> python -I -m pip install wheel
.pkg: freeze> python -m pip freeze --all
.pkg: packaging==23.2,pip==23.3.1,setuptools==68.2.2,setuptools-scm==8.0.4,typing_extensions==4.8.0,wheel==0.41.2
.pkg: prepare_metadata_for_build_wheel> python /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: build_sdist> python /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
towncrier-check: install_package_deps> python -I -m pip install 'matplotlib>=3.0.3' pre-commit==3.5.0 pytest==7.4.3 'setuptools-scm[toml]==8.0.4' 'toga>=0.3.1' tox==4.11.3
towncrier-check: install_package> python -I -m pip install --force-reinstall --no-deps /Users/runner/work/toga-chart/toga-chart/.tox/.tmp/package/1/toga-chart-0.1.2.dev115+g0db2074.tar.gz
towncrier-check: freeze> python -m pip freeze --all
towncrier-check: cachetools==5.3.2,cfgv==3.4.0,chardet==5.2.0,click==8.1.7,click-default-group==1.2.4,colorama==0.4.6,contourpy==1.1.1,cycler==0.12.1,distlib==0.3.7,filelock==3.13.1,fonttools==4.43.1,identify==2.5.31,incremental==22.10.0,iniconfig==2.0.0,Jinja2==3.1.2,kiwisolver==1.4.5,MarkupSafe==2.1.3,matplotlib==3.8.1,nodeenv==1.8.0,numpy==1.26.1,packaging==23.2,Pillow==10.1.0,pip==23.3.1,platformdirs==3.11.0,pluggy==1.3.0,pre-commit==3.5.0,pyparsing==3.1.1,pyproject-api==1.6.1,pytest==7.4.3,python-dateutil==2.8.2,PyYAML==6.0.1,rubicon-objc==0.4.7,setuptools==68.2.2,setuptools-scm==8.0.4,six==1.16.0,toga==0.3.1,toga-chart @ file:///Users/runner/work/toga-chart/toga-chart/.tox/.tmp/package/1/toga-chart-0.1.2.dev115%2Bg0db2074.tar.gz#sha256=f16d68e9d736375426dab0698d324323a546e16f9b738c4143365338318bb72d,toga-cocoa==0.3.1,toga-core==0.3.1,towncrier==22.12.0,tox==4.11.3,travertino==0.3.0,typing_extensions==4.8.0,virtualenv==20.24.6,wheel==0.41.2
towncrier-check: commands[0]> python -m towncrier.check --compare-with=origin/main
Looking at these files:
----
1. /Users/runner/work/toga-chart/toga-chart/setup.cfg
----
No new newsfragments found on this branch.
towncrier-check: exit 1 (0.34 seconds) /Users/runner/work/toga-chart/toga-chart> python -m towncrier.check --compare-with=origin/main pid=4390
.pkg: _exit> python /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
  towncrier-check: FAIL code 1 (28.93=setup[28.60]+cmd[0.34] seconds)
  evaluation failed :( (29.01 seconds)
Error: Process completed with exit code 1.

Additional context

No response

[rmartin16]

Hello @ahonnecke, The CI step is erroring because the PR doesn't have a "news fragment" (aka "changenote"). Add a file named 60.misc.rst to the changes directory with content describing your changes; then add that file to the PR and CI will succeed for this step.

Looks like these details are missing from toga-chart's contributing documentation but it would look something like this: https://briefcase.readthedocs.io/en/stable/how-to/contribute-code.html#add-change-information-for-release-notes

[ahonnecke]

@rmartin16 Thank you! would it be helpful for me to either add that to the contibuting docs, or add a pre-commit step that checks for said file? Or both?

[rmartin16]

You're welcome. A pre-commit step is probably overkill because tox -e towncrier takes too long....and tbh, for most people from what I've seen, if they don't know a towncrier note is needed, they also probably don't have pre-commit set up for their local repo.

As for toga-chart's documentation, it probably wouldn't hurt to copy what's in Briefcase's docs....although, it looks like the docs here could use a little TLC; for instance, I see a reference to Python 3.5 and it isn't using the furo theme.

[ahonnecke]

I thought that you ran pre-commit in CI. Perhaps I mis-remember but I saw it fail. Perhaps adding linting in CI would be helpful?

[rmartin16]

Correct; pre-commit does run in CI....although, as the name "pre-commit" implies, it is intended to run before the commit is made :) ideally, in the user's environment when they are creating the commit. Running pre-commit install in the local repo directory accomplishes this.

[ahonnecke]

I think that the current setup (where when someone tries to open a PR and it fails with a mystery error from town crier) could possible be improved if the PR were to fail with an error that is less obtuse. Something like "you have to add a file to the changes directory with content describing your changes"

Since pre-commit runs in github actions, I was suggesting to add a custom step to pre-commit that will fail with a clear error if the PR doesn't have the required changeset file.

I don't understand how "the user can run pre-commit locally" has any bearing.

[rmartin16]

I agree the error state is confusing; tbh, most people's first PR gets stuck here because they don't understand what is necessary. This is even true for the other repos that have thorough contributing docs and mention this explicitly.

That said, once you understand the error, you don't get stuck on it again later....you simply add the changenote. Given it's a one time error, the incentive for actively detecting this state and reporting it more accurately is decreased.

Although, if it could be easily detected or reported on, then a balance with the decreased incentive may be reached....but this is non-trivial to meaningfully detect and report without introducing a bunch of complicated logic that's likely to introduce a maintenance burden of its own.

Given all that....it may be worthwhile to add something to the tox config for towncrier to drop a link to the appropriate documentation saying what's necessary when the towncrier check fails. I've considered this in the past but haven't taken the next step to actually implement it.

[rmartin16]

FYI. Proposed a more informative error for the towncrier failure: https://github.com/beeware/.github/pull/63