Auto format code on non-protected branches. by niccokunzmann · Pull Request #1208 · collective/icalendar

niccokunzmann · 2026-02-17T16:10:59Z

Closes issue

Contributors might push code that is badly formatted and then need to work more before the change can be accepted.
If you edit a file through the web UI, you do not have code formatting available. This automatically applies certain fixes.

Description

This automatically pushes commits with formatted code to the pull requests.

branches like .x and main and tags are exempt.

Checklist

I've added a change log entry to CHANGES.rst.
I've added or updated tests if applicable.
I've run and ensured all tests pass locally by following Run tests.
I've added or edited documentation, both as docstrings to be rendered in the API documentation and narrative documentation, as necessary.

Additional information

The commit 98bca82 should be reverted automatically.

📚 Documentation preview 📚: https://icalendar--1208.org.readthedocs.build/

Copilot

Pull request overview

Adds a GitHub Actions workflow intended to auto-format Python code with Ruff on pull requests by committing formatting changes back to the PR branch.

Changes:

Add .github/workflows/ruff-format.yml to run ruff format / ruff check --fix and auto-commit results.
Add trailing blank lines to src/icalendar/__init__.py (likely accidental/formatter artifact).

Reviewed changes

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

File	Description
`src/icalendar/__init__.py`	Adds trailing blank lines at EOF.
`.github/workflows/ruff-format.yml`	New workflow to format/lint-fix code and push commits back to PRs.

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

niccokunzmann · 2026-02-17T17:04:16Z

I think that this would be a nice feature but it needs to run on the repository of the person committing. So, maybe PR is not the right context but push.

coveralls · 2026-02-17T17:44:29Z

Pull Request Test Coverage Report for Build 22106041453

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 98.091%

Totals
Change from base Build 22089382994:	0.0%
Covered Lines:	11382
Relevant Lines:	11597

💛 - Coveralls

coveralls · 2026-02-17T17:44:42Z

Pull Request Test Coverage Report for Build 22106548673

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 98.091%

Totals
Change from base Build 22089382994:	0.0%
Covered Lines:	11382
Relevant Lines:	11597

💛 - Coveralls

coveralls · 2026-02-17T17:44:43Z

Pull Request Test Coverage Report for Build 22277551569

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 98.081%

Totals
Change from base Build 22274921870:	0.0%
Covered Lines:	11418
Relevant Lines:	11635

💛 - Coveralls

See https://icalendar.readthedocs.io/en/stable/contribute/development.html#code-format

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

niccokunzmann · 2026-02-17T17:51:14Z

The commit d7128bf was automatically created on the fork's repository and pushed with the formatting changes.

SashankBhamidi · 2026-02-17T18:31:50Z

Okay, nice. I'll take a look at the changes in the morning.

I'm going to rerequest another review from copilot.

Why is coveralls spamming?

Copilot

Pull request overview

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

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Steve Piercy <web@stevepiercy.com>

niccokunzmann · 2026-02-17T20:49:28Z

@stevepiercy On the matter of confusion, I would try it out and get feedback. If it is an issue, it can be solved.

For now, this would ease this PR: #1205

stevepiercy · 2026-02-17T21:23:00Z

First-time contributors who are unfamiliar with git are not likely to disclose problems with not being able to push a commit to a branch that adds another commit from somewhere else. There's nothing automatically that notifies us of a problem, and it leads to the PR getting abandoned.

In Plone, I often see strange behavior, including closing and opening multiple PRs, not creating a new branch for each pull request so that all changes pile up onto their most recent PR, and other common mistakes with git. Sometimes pointing the user to our docs for how to use git helps, but they have to read and understand them first. When I can identify the cause, one of these sections sometimes helps.

SashankBhamidi

A few things:

Lines 21-22: github.event.pull_request.* is null on push events. Checkout falls back to defaults (verified in checkout's input-helper.ts: core.getInput('repository') || context.repo). These lines do nothing, should be removed.
Copilot correctly flagged lines 21-22 and the ruff check --fix scope. Worth reconsidering those.

Co-authored-by: Sashank <hello@sashank.wiki>

niccokunzmann · 2026-02-18T10:31:53Z

+      pull-requests: write
+    name: Format the code with ruff and push the changes
+    runs-on: ubuntu-latest
+    if: github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/tags/') && !(startsWith(github.ref, 'refs/heads/') && endsWith(github.ref, '.x')) && !startsWith(github.ref, 'refs/heads/release')


The ! at the start was ignored by GitHub. I re-ordered the condition to not start with that.

In the downloaded log zip, there is a system.txt showing:

( true && ( ('refs/heads/auto-format' != 'refs/heads/main') && !startsWith('refs/heads/auto-format', 'refs/tags/') && !( startsWith('refs/heads/auto-format', 'refs/heads/') && endsWith('refs/heads/auto-format', '.x') ) && !startsWith('refs/heads/auto-format', 'refs/heads/release')))

That should exclude main, all .x branches and tags as well as the release-... branches created by the maintenance tasks of releasing.

stevepiercy · 2026-02-18T10:31:47Z

+      pull-requests: write
+    name: Format the code with ruff and push the changes
+    runs-on: ubuntu-latest
+    if: ! startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/main' && ! (startsWith(github.ref, 'refs/heads/') && endsWith(github.ref, '.x')) && ! startsWith(github.ref, 'refs/heads/release')


What is the intent here? It would be good to add a comment and possibly a link to a reference to explain the intent of this statement, as its arguments aren't really clear. That will help make sure what you want is what you get, and help other people understand it.

heh, and you just updated it, but my suggestion stands.

Yes, comments and purpose are in 73a97fe. Please have a look.

stevepiercy · 2026-02-18T10:54:09Z

+    #
+    # This workflow EXCLUDES:
+    #
+    # - branch "main"


This sounds like I can fork to stevepiercy/icalendar, push to my main and it won't run.

Then when I open a PR against collective/icalendar on its main branch, it still won't run.

Is that what you want?

Same for the remaining exclusions below.

I think, that is a good start.

It can be extended to format main on other repos as that is where first -timers often start like in #1205. I would do that another time, and it requires more consideration.

I think, I prefer it if main stays main and can be fast-forwarded to collective/calendar's main.

Honestly, I think automatically rewriting pull requests is a bad idea all around. Pull requests should be intentional and deliberate, which is something only humans can do.

That is ok. I will not talk about the value. I see value. I would like to see if this is technically correct. Then, I will merge it or someone else does it and we see if it makes sense. We can remove it if it gets in the way of contribution.

Honestly, I think automatically rewriting pull requests is a bad idea all around. Pull requests should be intentional and deliberate, which is something only humans can do.

+1

SashankBhamidi

This may be merged, has value, but I'm not too sure if first time contributors can keep up with pulling changes from branch to continue working without conflicts.

niccokunzmann · 2026-02-22T12:55:30Z

Yes, that could be an issue. I guess, we will see if they can cope if we see the formatting commits.

stevepiercy · 2026-02-22T13:00:06Z

It's not just first-timer contributors, but all contributors, including me. I personally hate it. Really, REALLY hate it.

niccokunzmann · 2026-02-23T20:20:34Z

It's not just first-timer contributors, but all contributors, including me. I personally hate it. Really, REALLY hate it.

Yes, I think that is clear! Thanks for the perspective. It is not merged. I am still thinking. #1216 also contributes to improving the work flow for non-formatted PRs.

niccokunzmann · 2026-02-23T22:22:03Z

Something like this could help:

https://github.com/orgs/community/discussions/25100

It would be nice to just comment "Please format" and that PR gets formatted.

I mean... One part of it is ease, the other one being perfectionist and the other one is having fun tinkering and exploring.

stevepiercy · 2026-02-25T12:49:37Z

It would be nice to just comment "Please format" and that PR gets formatted.

Yes, if it's not 100% automated, but requires a human to intervene and approve the request after they review the proposed changes, then that would be OK. However, I'm afraid that most people will just click the "Go ahead, IDGAF" button and not review the proposed changes. There's a distinct advantage to forcing people to think and make intentional changes, instead of clicking the "easy" button.

niccokunzmann · 2026-04-04T17:45:00Z

This is stale. I will close this now. This can be taken up and changed when needed.

niccokunzmann added 3 commits February 17, 2026 17:06

Auto format code with ruff

054e93d

modify __init__.py to be formatted

98bca82

Check for changes

8730f15

niccokunzmann requested review from SashankBhamidi, angatha and stevepiercy as code owners February 17, 2026 16:11

niccokunzmann added 2 commits February 17, 2026 17:25

Use a different action to push the git commit

155a7fd

do not fail if changes are there

21c2569

SashankBhamidi requested a review from Copilot February 17, 2026 16:36

Copilot started reviewing on behalf of SashankBhamidi February 17, 2026 16:38 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Use another action step to push changes

20b31c2

niccokunzmann added 2 commits February 17, 2026 18:40

Run action on push of branch

1ca1d99

format on push

7b3561d

niccokunzmann marked this pull request as draft February 17, 2026 17:42

niccokunzmann and others added 4 commits February 17, 2026 18:45

do not specify branch

9f2bc96

use ref_name for branch

92a4a8d

Format with ruff

d7128bf

See https://icalendar.readthedocs.io/en/stable/contribute/development.html#code-format

Update .github/workflows/ruff-format.yml

b400a99

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

niccokunzmann marked this pull request as ready for review February 17, 2026 17:49

log changes

e577bab

niccokunzmann changed the title ~~Auto format~~ Auto format code on non-protected branches. Feb 17, 2026

SashankBhamidi requested a review from Copilot February 17, 2026 18:32

Copilot started reviewing on behalf of SashankBhamidi February 17, 2026 18:32 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Comment thread .github/workflows/ruff-format.yml Outdated

Comment thread .github/workflows/ruff-format.yml Outdated

Comment thread .github/workflows/ruff-format.yml Outdated

Comment thread .github/workflows/ruff-format.yml

Comment thread .github/workflows/ruff-format.yml

niccokunzmann force-pushed the auto-format branch from 28e991a to e577bab Compare February 17, 2026 20:23

stevepiercy requested changes Feb 17, 2026

View reviewed changes

Comment thread CHANGES.rst Outdated

niccokunzmann and others added 2 commits February 17, 2026 21:46

fix push action

7a30d10

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update CHANGES.rst

8202590

Co-authored-by: Steve Piercy <web@stevepiercy.com>

niccokunzmann added 2 commits February 17, 2026 22:07

Merge branch 'main' into auto-format

0837313

Fix run condition of auto formatting

f856122

Merge branch 'main' into auto-format

5100fee

SashankBhamidi requested changes Feb 18, 2026

View reviewed changes

Comment thread .github/workflows/ruff-format.yml Outdated

Comment thread .github/workflows/ruff-format.yml Outdated

Comment thread .github/workflows/ruff-format.yml

Comment thread .github/workflows/ruff-format.yml

Comment thread CHANGES.rst Outdated

niccokunzmann and others added 3 commits February 18, 2026 09:58

remove unused repository scope and add commit step

1ce254c

Apply suggestions from Sashank

1272cf3

Co-authored-by: Sashank <hello@sashank.wiki>

correct run condition

c9aaa8c

niccokunzmann commented Feb 18, 2026

View reviewed changes

stevepiercy reviewed Feb 18, 2026

View reviewed changes

Document the auto-formatting workflow

73a97fe

stevepiercy reviewed Feb 18, 2026

View reviewed changes

SashankBhamidi previously approved these changes Feb 22, 2026

View reviewed changes

Merge branch 'main' into auto-format

3c119b5

niccokunzmann dismissed SashankBhamidi’s stale review via 3c119b5 February 22, 2026 12:54

niccokunzmann mentioned this pull request Feb 23, 2026

Output the required formatting changes #1216

Merged

4 tasks

niccokunzmann closed this Apr 4, 2026

Uh oh!

Conversation

niccokunzmann commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Closes issue

Description

Checklist

Additional information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niccokunzmann commented Feb 17, 2026

Uh oh!

coveralls commented Feb 17, 2026

Pull Request Test Coverage Report for Build 22106041453

Details

💛 - Coveralls

Uh oh!

coveralls commented Feb 17, 2026

Pull Request Test Coverage Report for Build 22106548673

Details

💛 - Coveralls

Uh oh!

coveralls commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 22277551569

Details

💛 - Coveralls

Uh oh!

niccokunzmann commented Feb 17, 2026

Uh oh!

SashankBhamidi commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niccokunzmann commented Feb 17, 2026

Uh oh!

stevepiercy commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SashankBhamidi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niccokunzmann Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevepiercy Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

stevepiercy Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

niccokunzmann Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

stevepiercy Feb 18, 2026

Choose a reason for hiding this comment

niccokunzmann commented Feb 17, 2026 •

edited

Loading

coveralls commented Feb 17, 2026 •

edited

Loading

stevepiercy commented Feb 17, 2026 •

edited

Loading

niccokunzmann Feb 18, 2026 •

edited

Loading

niccokunzmann Feb 18, 2026 •

edited

Loading

niccokunzmann commented Feb 23, 2026 •

edited

Loading