CI: add a workflow to show flake8/mypy violations in modified files of a PR #1513
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hm, I'm not sure it's a good idea to merge this. I think we could make failures not block merging, but it would still show up with a red X in the overview, so there is a good chance that would disrupt the review workflow. Having to fix formatting of hundreds of lines of code to get a small change to pass does not sound like the proper way either (unless we go in now and auto-format all of core now, breaking all current core PRs in the process). Also worlds are (currently) not forced to follow (all of) pep8, opening a PR with this action would break that promise, but they could be excluded in the action. |
|
Can this be limited to not just the changed file, but changed lines? |
I'm also not sure it's a good idea to merge this.
flake8 used to have a feature that tried to do this, but it was removed because it just doesn't work. Changing one line can cause a violation on an unchanged line. |
el-u
changed the title
|
…rkflow on push event; modify a file to trigger the push workflow
|
black-sliver
approved these changes
May 20, 2023
Joethepic
added a commit
to Joethepic/Archipelago
that referenced
this pull request
Jun 5, 2023
* Added undertale world files * Tried to fix generation * Fixed generation of Undertale * added Undertale Client * Finished making everything * fixed an item being in the wrong location * Fixed having same item ids as another game * Fixed some duplicate item ids and removed a connection that might cause problems * Added partially automatic patching of Undertale * fixed not building the client * Made changes to Undertale AP * Added an option to remove Temy Armor from the Undertale pool * updated the undertale patch * Added the setups folder to gitignore * updated the undertale patch * Made the plot be location based items * Fixed items that have a negative id * Made changes and additions to Undertale * Added features to Undertale * Added Soul Hunt to Undertale * Fixed the Undertale rules * Fixed another bug * Forgot to fix this Undertale * Added missing client variables * Made UndertaleClient not have copies of CommonClient * fixed hotland logic rules for Undertale * Added deathlink * Fixed some stuff with deathlink * Updated the Undertale patch * Fixed deathlink and local items * Fixed deathlink again in undertale * Fixed a permission error bug with opening files * Made online mode and deathlink a toggle command * Added Progressive Plot to undertale * Updated the patch for Undertale * Added LV rando to Undertale * hopefully made Undertale online mode more stable * Fixed LOVE rando not actually getting told to Undertale * Added a secret feature to Undertale * Added locations for Undertale plot * Added All Routes setting to Undertale * Fixed Undertale rules * Added Area rando to Undertale AP * Fixed a crash in Undertale with starting a new save from nothing * Added stats rando * Almost fixed Undertale generation * Fixed Undertale Stat Rando * Fixed one thing with undertale stat rando * Added progressive weapons and armor to Undertale * Made progressive plot change based on area * Undertale: Updated the patch * Fixed some things with the Undertale Client * Fixed a bunch of stuff with Undertale * attempt at fix * True Fix Undertale * Wow, fixed Undertale * Fixed stuff with Undertale, and made some items marked as useful * Updated the patch for Undertale * Fixed some stuff with Undertale ap * Finally made Undertale have an auto patch feature * Added docs to Undertale * Fixed something with the auto patching of undertale * Actually got the docs right for Undertale * Fixed the Docs for Undertale * Changed gold amount * Fixed data_version for Undertale * Fixed some stuff with Undertale * Fixed some Undertale stuff * Fixed the Undertale init.py file * Fixed some generation errors with Undertale * Fixed the Undertale Client freezing * Made the auto patcher create the Custom Sprites folder * Updated Undertale * Fixed Undertale bugs * Made some changes to UndertaleClient * Fixed some stuff * Fixed some Undertale bugs * Fixed Undertale AP bugs * Final changes * Changed docs some, and fixed a bug with photoshop flowey * Some more fixes * Made a change to a line in UT docs * Forgot to fix the data version * Fixed some Undertale bugs * Made requested changes * Fixed it * Fixed broken stuff * Fixed pacifist route * Fixed player hitbox * Fixed some stuff with clearing files * Fully made Undertale work as an apworld file * Actually fixed undertale client * Forgot to update the patch * fixed some stuff * Actually updated the patch * Made suggested changes * Quality of life change * Updated the Undertale Client launcher component to use the new method * Made requested changes, and removed unused install argument * Missed two lines with the removal of the unused install argument * KH2: AntipointReset (ArchipelagoMW#1815) * Core: skip ModuleUpdate in subprocess * ALttP: fix dungeon fill failures properly (ArchipelagoMW#1812) * Blasphemous: Fixed logic errors in WotHP * CI: add a workflow to show flake8/mypy violations in modified files of a PR (ArchipelagoMW#1513) * CI: add a workflow to show flake8 violations in modified files of a PR * modify a file to trigger the lint check * CI: add a workflow to show mypy violations in modified files of a PR * modify a file to trigger the type check * Split flake8 and mypy into two parallel jobs; run a variant of the workflow on push event; modify a file to trigger the push workflow * fail the task if there are syntax errors; remove old lint workflow * LADX: Add --no-magpie argument for disabling magpie bridge (ArchipelagoMW#1788) * Subnautica: move mod exports to own module * Main: add __all__ and change wrong imports (ArchipelagoMW#1824) * Main: add __all__ and change wrong imports * Adjusters: fix __version__ import * Logging: make sure level is applied for websockets * Core: update modules * LttP: deterministic shop_shuffle * LttP: extract Dungeon and Boss from core (ArchipelagoMW#1787) * Docs: Update world api excluded/priority locations description (ArchipelagoMW#1807) * Update world api doc Changed the description of excluded and priority locations to match how they appear in other places such as the options api doc * Update world api.md * CI: treat all files as modified on new branches (ArchipelagoMW#1826) * The Messenger: override start_inventory description (ArchipelagoMW#1695) * The Messenger: override start_inventory description * use StartInventoryPool directly * WebHost: index columns used by landing page. * WebHost: add game to template export * Core: log race mode enabled * DLCQuest: Fix Documentation Broken Link * [Blasphemous] Various logic fixes (ArchipelagoMW#1830) This makes a few changes to logic to better match the 1.3 rando's logic. This fixes instances where the wrong items were expected, fixes a typo of "Lorqiana", moves the expert logic on "PotSS: Second area ledge" to only apply if on expert, and adds a new route to "DC: Mea Culpa altar" via Linen of Golden Thread + Three Gnarled Tongues * [SM] Minor update to link in Options.py (ArchipelagoMW#1831) * Core: clean up BaseClasses a bit (ArchipelagoMW#1731) * WebHost: use Py3.11 compatible ponyorm * LttP: fix patching crash if old always_apply adjuster settings were applied * Stardew valley: Fix package and imports for apworld linux (ArchipelagoMW#1842) - Fix csv load to use explicitly imported self package instead of keyword __package__ - Fix init.py having a relative import to outside of the apworld * Wargroove: Fixed commander.json file never being closed by the mod (ArchipelagoMW#1841) The Wargroove mod didn't close the commander.json's file handle. The Wargroove mod will now close that file handle. The change for the mod can be viewed here: FlySniper/WargrooveArchipelagoMod@fc9aeb3 The change can be verified as present in this repository by viewing the binary data in the modAssets.dat file and searching for "commander.json" * SMZ3 decoding fix (ArchipelagoMW#1847) * Zillion: cache key includes gun requirement (ArchipelagoMW#1846) The key for the logic cache was missing some important information, so it was yielding a cache hit when it should have been a miss. --------- Co-authored-by: jonloveslegos <jonloveslegos@gmail.com> Co-authored-by: Mewlif <68133186+jonloveslegos@users.noreply.github.com> Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com> Co-authored-by: JaredWeakStrike <96694163+JaredWeakStrike@users.noreply.github.com> Co-authored-by: Fabian Dill <fabian.dill@web.de> Co-authored-by: espeon65536 <81029175+espeon65536@users.noreply.github.com> Co-authored-by: Exempt-Medic <60412657+Exempt-Medic@users.noreply.github.com> Co-authored-by: el-u <109771707+el-u@users.noreply.github.com> Co-authored-by: Cybrou <106977036+Cybrou@users.noreply.github.com> Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com> Co-authored-by: alwaysintreble <mmmcheese158@gmail.com> Co-authored-by: axe-y <58866768+axe-y@users.noreply.github.com> Co-authored-by: ScootyPuffJr1 <77215594+ScootyPuffJr1@users.noreply.github.com> Co-authored-by: agilbert1412 <alexgilbert@yahoo.com> Co-authored-by: FlySniper <brajcan@hotmail.com> Co-authored-by: lordlou <87331798+lordlou@users.noreply.github.com> Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
kindasneaki
pushed a commit
to kindasneaki/Archipelago
that referenced
this pull request
Jun 28, 2023
…f a PR (ArchipelagoMW#1513) * CI: add a workflow to show flake8 violations in modified files of a PR * modify a file to trigger the lint check * CI: add a workflow to show mypy violations in modified files of a PR * modify a file to trigger the type check * Split flake8 and mypy into two parallel jobs; run a variant of the workflow on push event; modify a file to trigger the push workflow * fail the task if there are syntax errors; remove old lint workflow
FlySniper
pushed a commit
to FlySniper/Archipelago
that referenced
this pull request
Nov 14, 2023
…f a PR (ArchipelagoMW#1513) * CI: add a workflow to show flake8 violations in modified files of a PR * modify a file to trigger the lint check * CI: add a workflow to show mypy violations in modified files of a PR * modify a file to trigger the type check * Split flake8 and mypy into two parallel jobs; run a variant of the workflow on push event; modify a file to trigger the push workflow * fail the task if there are syntax errors; remove old lint workflow
Jouramie
pushed a commit
to Jouramie/Archipelago
that referenced
this pull request
Feb 28, 2024
…f a PR (ArchipelagoMW#1513) * CI: add a workflow to show flake8 violations in modified files of a PR * modify a file to trigger the lint check * CI: add a workflow to show mypy violations in modified files of a PR * modify a file to trigger the type check * Split flake8 and mypy into two parallel jobs; run a variant of the workflow on push event; modify a file to trigger the push workflow * fail the task if there are syntax errors; remove old lint workflow
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this fixing or adding?
Adds a new GitHub workflow that tries to identify which Python files have been changed by a PR. Only these files will then be checked with flake8 and an error will be raised if violations are found.
This should be useful for pull request review because it makes it easier to isolate issues that might be introduced due to the changed code. The existing lint workflow is not suitable for this task because it checks all existing Python files, which leads to a multitude of errors stemming from the untouched, existing code, making it impossible to judge the effect of only the new code.
The new solution is not perfect. As soon as a single line in a file is changed, the whole file will have to be run through the linter, meaning it will also print all previously existing violations from that file. This is not something that can be avoided. The linter always has to check the whole file, since changing one line could cause a violation that is located on an unchanged line. As such, the usefulness of this feature will be maximized on new worlds, but it will be somewhat limited when applied to PRs that change core code. (That is, until all code code is cleaned up and free of violations...)
How was this tested?
By modifying an otherwise unrelated Python file as part of this PR and verifying that this file (and only this file) will by checked by the linter.
If this makes graphical changes, please attach screenshots.