Skip to content

Use sophisticated pre-commit for validation#1071

Merged
mergify[bot] merged 4 commits into
ccfddl:mainfrom
juhannc:pre-commit
Jul 17, 2025
Merged

Use sophisticated pre-commit for validation#1071
mergify[bot] merged 4 commits into
ccfddl:mainfrom
juhannc:pre-commit

Conversation

@juhannc

@juhannc juhannc commented Jul 16, 2025

Copy link
Copy Markdown
Contributor

This PR updates the pre-commit pipeline to utilize the sophisticated pre-commit framework.

Major changes:

  • Use a global .pre-commit-config.yaml file for the pre-commit settings (f4f2ce3)
  • Move the previous validate pipeline into the global pre-commit file using a local repo (f4f2ce3)
  • Adapt the GitHub workflows to use the new framework (9ee3719)
    • I removed the validation from the pipelines and created a new workflow only for pre-commit. This seperates errors and is easier to maintain
  • Add additional pre-commit repos to run basic checks (aa90828)

Minor changes:

  • Run the new re-commit pipeline formatting quite a lot of files with no changes to the website (it's just formatting) (c3c33fa)

@mergify

mergify Bot commented Jul 16, 2025

Copy link
Copy Markdown
Contributor

Welcome to ccf-deadlines!
We're delighted to have you onboard (p≧w≦q)

@mergify mergify Bot added the updateconf update conference deadline label Jul 16, 2025
@mergify mergify Bot requested a review from a team July 16, 2025 10:16
@jacklightChen

Copy link
Copy Markdown
Member

We appreciate your efforts in enhancing the workflow. Plz wait patiently for the review.

Comment thread accept_rates/AI/ecai.yml
submitted: 2344
accepted: 547
str: 23.3%(547/2344 24')
rate: 0.233361774744027303

@jacklightChen jacklightChen Jul 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just curious—if it's just formatting, why does the value change😂

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I didn't catch that. Apperently that's an issue of the underlying ruamel.yaml used by macisamuele/language-formatters-pre-commit-hooks

Running:

import sys

import ruamel.yaml

yaml_str = """\
val: 0.233361774744027303
"""

print(yaml_str.replace("\n", ""))
yaml = ruamel.yaml.YAML()
data = yaml.load(yaml_str)
yaml.dump(data, sys.stdout)

outputs

val: 0.233361774744027303
val: 0.233361774744027306

I'll raise the issue with ruamel but in the mean time, I'll switch to another yaml formatter will rewrite the commit to ensure no data is lost.

@mergify mergify Bot requested a review from a team July 17, 2025 06:45
@juhannc

juhannc commented Jul 17, 2025

Copy link
Copy Markdown
Contributor Author

@jacklightChen I switched to another yaml formatter and redid the formatting. I also split the formatting into two commits. One for adding the new hooks (2ea24d6) and one for running the formatters for the first time (c047fe6). I hope it makes reviewing easier :)

PS: For some reason, I re-pushed the same wrong configuration at first. Everything should check out now. Unfortunately, the yaml formatter from Google doesn't remove unnecessary quotation marks, etc., but changing values is a no-go

@jacklightChen jacklightChen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mergify mergify Bot merged commit e37f671 into ccfddl:main Jul 17, 2025
3 checks passed
@mergify

mergify Bot commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

Great job!
Thank you @juhannc for your contribution!

@mergify mergify Bot added the ready-to-merge ready to merge the pr label Jul 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge ready to merge the pr updateconf update conference deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants