Skip to content

Conversation

@stonedDiscord
Copy link
Contributor

I realize this is in direct competition with #14107, I started work on this on saturday when I posted the question if this could be automated in the Discord server.

The action will post code reviews, much like a human reviwer would.
An example can be seen here: stonedDiscord#5

I went through https://docs.mamedev.org/contributing/cxx.html to come up with these rules.
It will detect

  • Missing newline at EOF
  • Invalid UTF-8
  • Wrong type of line ending
  • Trailing whitespaces
  • Uppercase hex literals
  • Single line block comments
  • Lowercase constants
  • Uppercase functions
  • Uppercase class names
  • Uppercase Enum names

It mostly uses regex to detect those, I haven't run into any false positives yet, but I'm on the fence about dropping the constants check because it will complain about variables that someone has marked const.

@cuavas
Copy link
Member

cuavas commented Sep 2, 2025

And ironically, it’s a Python script with no final newline…

@stonedDiscord
Copy link
Contributor Author

I'm aware that I get this constantly wrong, that's why I asked and then wrote this script.
I guess I'm simply not used to that style rule and I will probably get it wrong again.
The point of the script is to let newbies like myself deal with the review bot so maintainers have more time to review PRs that are ready for merging and don't get fed up or annoyed as much.
The CI doesn't block merging, same as with human reviews.

@stonedDiscord
Copy link
Contributor Author

stonedDiscord commented Sep 2, 2025

I've changed it to use no external dependencies and to post a single review with multiple comments, fixing any rate limit issues before they appear.

I've tested the style checker against #14020 here:
stonedDiscord#7

@stonedDiscord
Copy link
Contributor Author

Added checks:

  • SPDX header
  • Include order
  • ROM region

commit 328909c
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 18:53:03 2025 +0100

    delete old comments

commit 19076db
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 18:46:03 2025 +0100

    resort

commit ffcc8dd
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 18:42:25 2025 +0100

    move logmacro last

commit 26fac09
Merge: 2543222 525dcb9
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 18:22:27 2025 +0100

    Merge remote-tracking branch 'upstream/master' into diy_review

commit 2543222
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 18:13:34 2025 +0100

    do filter in py script

commit bbbf70c
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 12:02:08 2025 +0100

    review comments

commit 31a2a09
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 11:38:33 2025 +0100

    delete step

commit e893d84
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 11:37:59 2025 +0100

    commit id

commit 362fb22
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 11:23:22 2025 +0100

    natural sort

commit 4432bb8
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 10:27:18 2025 +0100

    Update bot user check in GitHub Actions workflow

commit 4519df8
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 10:16:31 2025 +0100

    delete comments

commit ff8c06e
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 10:08:41 2025 +0100

    Improve sorting by stripping trailing numbers

commit d507186
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 09:58:27 2025 +0100

    use code comments instead

commit 1820f22
Author: stonedDiscord <[email protected]>
Date:   Wed Nov 5 09:49:02 2025 +0100

    dismiss

commit 0a65f6c
Author: stonedDiscord <[email protected]>
Date:   Sun Nov 2 01:31:25 2025 +0100

    Filter

commit c4f281a
Author: stonedDiscord <[email protected]>
Date:   Fri Oct 31 21:29:31 2025 +0100

    add token

commit a05626a
Author: stonedDiscord <[email protected]>
Date:   Fri Oct 31 20:52:09 2025 +0100

    grant permission to write a comment

commit fc86e48
Author: stonedDiscord <[email protected]>
Date:   Fri Oct 31 18:08:00 2025 +0100

    seperate it out

commit 550840d
Author: stonedDiscord <[email protected]>
Date:   Fri Oct 31 17:47:09 2025 +0100

    rom regions

commit dc6accb
Author: stonedDiscord <[email protected]>
Date:   Fri Oct 31 17:40:19 2025 +0100

    include order

commit 8e8b793
Author: stonedDiscord <[email protected]>
Date:   Fri Oct 31 17:26:53 2025 +0100

    spdx

commit 786f472
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 19:40:01 2025 +0200

    ugly log

commit 71854d4
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 19:32:25 2025 +0200

    one line please

commit a5e970e
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 19:29:26 2025 +0200

    one big review

commit 329f1de
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 19:14:29 2025 +0200

    use error

commit 9b77efb
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:59:13 2025 +0200

    use 3 dot compare and fetch more data

commit 10f9c8b
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:47:26 2025 +0200

    deprecated

commit fcd1926
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:46:11 2025 +0200

    use my HEAD for a bit

commit 27e5e23
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:42:44 2025 +0200

    try yet another method

commit db51b69
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:38:12 2025 +0200

    missing token

commit 795a96d
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:36:25 2025 +0200

    use gh command over git

commit a39ff7f
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 18:24:37 2025 +0200

    fix changed files?

commit b58b207
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 15:57:41 2025 +0200

    post the warnings from the script

commit 0f672a4
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 15:50:31 2025 +0200

    WIP

commit 9641ecc
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 15:37:46 2025 +0200

    remove ext deps

commit 83d2cb4
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 14:31:39 2025 +0200

    the irony

commit 719cab9
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 00:16:07 2025 +0200

    wording

commit 22504bc
Author: stonedDiscord <[email protected]>
Date:   Tue Sep 2 00:05:15 2025 +0200

    disable suggestions

commit 101e46b
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 18:03:19 2025 +0200

    HUH

commit a40e79f
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 18:02:00 2025 +0200

    do not fix whitespaces and check other files

commit 16615a4
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 17:57:39 2025 +0200

    check utf8, line endings and whitespaces

commit 985916f
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 17:53:39 2025 +0200

    lst

commit 2fd7ef6
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 17:41:17 2025 +0200

    didnt i remove this already

commit 7537597
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 17:27:53 2025 +0200

    oops

commit ebebdb7
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 17:26:54 2025 +0200

    consider hpp files

commit 5f4071b
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 17:11:05 2025 +0200

    newline fix was merged in feb

commit 50144d7
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 16:47:05 2025 +0200

    tool_name works different

commit edd06e0
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 16:31:26 2025 +0200

    offer to fix the newline

commit c9c1220
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 13:35:11 2025 +0200

    less strong wording

commit e2a5502
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 13:33:43 2025 +0200

    filter files in script

commit 2e890a6
Author: stonedDiscord <[email protected]>
Date:   Mon Sep 1 13:20:58 2025 +0200

    license

commit 3046c29
Author: stonedDiscord <[email protected]>
Date:   Sun Aug 31 20:23:57 2025 +0200

    pretty useless

commit 85922c9
Author: stonedDiscord <[email protected]>
Date:   Sun Aug 31 20:15:44 2025 +0200

    use a library

commit 409f388
Author: stonedDiscord <[email protected]>
Date:   Sun Aug 31 20:05:15 2025 +0200

    try another way

commit b80e5f4
Author: stonedDiscord <[email protected]>
Date:   Sat Aug 30 20:16:25 2025 +0200

    run it

commit 37e7389
Author: stonedDiscord <[email protected]>
Date:   Sat Aug 30 20:10:52 2025 +0200

    get filenames

commit c8370b5
Author: stonedDiscord <[email protected]>
Date:   Sat Aug 30 20:03:27 2025 +0200

    bad indentation

commit b35a924
Author: stonedDiscord <[email protected]>
Date:   Sat Aug 30 19:57:57 2025 +0200

    also check mame.lst

commit bd82558
Author: stonedDiscord <[email protected]>
Date:   Sat Aug 30 19:12:49 2025 +0200

    free styler
so it doesnt send 20 emails per review
EOF for good luck
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.

3 participants