Opendream-detected Errors, More Harddels, Slight CI Improvements #3572
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
FalloutFalcon
approved these changes
Oct 16, 2024
FalloutFalcon
approved these changes
Oct 16, 2024
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
5d5d935 to
23d0f49
Compare
Merged
west3436
reviewed
Oct 17, 2024
west3436
reviewed
Oct 17, 2024
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
063b799 to
bba2f63
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
e2f03cc to
21647ea
Compare
This PR does a few things. The main goal was to have it annotate the code like we do for `Run Dreamchecker`, which it now does. The code for this was mostly shamelessly ripped from the very same process I just mentioned, but stripped down a lot more for the task. Since I had to write a lot more code to handle the way we get warnings and the like from OpenDream, I didn't fold both into one program.   It works! Ta-da. Anyways, in order to facilitate the code annotation, we need to use a Python script. This Python script means that we need to use the bootstrap, and the bootstrap is best used with the bootstrap cache. I then realized that we should just really fold this special CI runner into the Run Linters workflow anyways, instead of having it on its own runner (since it'll cut into the number of runners we have available at a time anyways). I think it's a good way to save precious CPU cycles on our dying Earth, and the issues are now visible enough to the average coder (in the `Files Changed` tab) to where I feel pretty good about doing this. There's a bit of weirdness and jank in the Python program, but it's all done for a reason I assure you (it's always silly when stuff breaks silently and no one knows to fix it until years later). You can look an example of a failing run here: https://github.com/san7890/bruhstation/actions/runs/8303759645/job/22728427623#step:17:14
Co-authored-by: PowerfulBacon <26465327+PowerfulBacon@users.noreply.github.com>
Start gate means we only need to check `[ci skip]` once Completion gate means we need only one status check in our rulesets.
This sets concurrency at the workflow level, which should provide a more concise way to eject if a new commit is entered https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency I've also added timeouts that are between 5-15 minutes per test. Default is 360 minutes without this
This PR removes the "Annotate Lints" job step and merges it with the "Run Linters" step above. To achieve this, I wrote a python script that should be identical to the action. I even added the ability to read the output from a file to the script if we ever needed that, but I decided to have the job step pipe the output into the script instead. It always bugged me a bit that we had to check the results for a separate step if we wanted to see the linter results for dm code. I also noticed a few people getting confused as to why their CI failed on linters. Turns out that the action is just a few lines that match the dreamchecker output and reformat it to a format that GitHub can annotate code with. It's so brain dead simple that it shouldn't need to be a whole new step, and for the previous two reasons. not playerfacing
Splits the big "Run Linters" step into multiple steps. Also since all of these steps are independent of eachother, I've made them all run regardless of if the job is currently failing. <details> <summary>Proof of testing:</summary> Fail in install tools, all linting steps are skipped: https://github.com/distributivgesetz/tgstation/actions/runs/6151628214/job/16692089726 Fail in Run DreamChecker, other steps continue to run: https://github.com/distributivgesetz/tgstation/actions/runs/6151664705/job/16692203569?pr=2 </details> <details> <summary>Pictured: me breaking CI for a day</summary> https://github.com/tgstation/tgstation/assets/47710522/ea12ad30-2b69-4aa3-9642-7d0818eab2d1 </details> Going through the Run Linters step has always been a slog. Finding an error is like finding a needle in a haystack. Seeing what command exactly went wrong is going to go a long way in helping people find out which linters have failed. nothing playerfacing
Modern browsers run the ready script before the DOM is loaded. Also future proofs us for when Lummox upgrades the web engine. --------- Co-authored-by: Aleksej Komarov <stylemistake@gmail.com>
Fix broken CI's
21647ea to
5e6153a
Compare
zimon9
pushed a commit
to zimon9/Shiptest-PR-testing
that referenced
this pull request
Oct 29, 2024
…ptest-ss13#3572) <!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request I didn't get them all in my last PR, apparently. Also throws in some minor tweaks for ambiguity and such detected by the OpenDream parser since I was messing around with that at the time. Also adds OpenDream linting to CI because why not. Ports: tgstation/tgstation#81892 (which is in turn from Para and Goon) tgstation/tgstation#82029 BeeStation/BeeStation-Hornet#11464 tgstation/tgstation#86510 tgstation/tgstation#83255 tgstation/tgstation#78225 tgstation/tgstation#78265 Fixes: shiptest-ss13#3530 ## Why It's Good For The Game Harddels still bad, OpenDream good <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. --> --------- Co-authored-by: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Co-authored-by: san7890 <the@san7890.com> Co-authored-by: PowerfulBacon <26465327+PowerfulBacon@users.noreply.github.com> Co-authored-by: Jordan Dominion <Cyberboss@users.noreply.github.com> Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com> Co-authored-by: distributivgesetz <distributivgesetz93@gmail.com> Co-authored-by: Aleksej Komarov <stylemistake@gmail.com>
MrCat15352
pushed a commit
to MrCat15352/MrCat
that referenced
this pull request
Dec 27, 2024
…ptest-ss13#3572) <!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> I didn't get them all in my last PR, apparently. Also throws in some minor tweaks for ambiguity and such detected by the OpenDream parser since I was messing around with that at the time. Also adds OpenDream linting to CI because why not. Ports: tgstation/tgstation#81892 (which is in turn from Para and Goon) tgstation/tgstation#82029 BeeStation/BeeStation-Hornet#11464 tgstation/tgstation#86510 tgstation/tgstation#83255 tgstation/tgstation#78225 tgstation/tgstation#78265 Fixes: shiptest-ss13#3530 Harddels still bad, OpenDream good <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. --> --------- Co-authored-by: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Co-authored-by: san7890 <the@san7890.com> Co-authored-by: PowerfulBacon <26465327+PowerfulBacon@users.noreply.github.com> Co-authored-by: Jordan Dominion <Cyberboss@users.noreply.github.com> Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com> Co-authored-by: distributivgesetz <distributivgesetz93@gmail.com> Co-authored-by: Aleksej Komarov <stylemistake@gmail.com>
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.
About The Pull Request
I didn't get them all in my last PR, apparently. Also throws in some minor tweaks for ambiguity and such detected by the OpenDream parser since I was messing around with that at the time.
Also adds OpenDream linting to CI because why not.
Ports:
tgstation/tgstation#81892 (which is in turn from Para and Goon)
tgstation/tgstation#82029
BeeStation/BeeStation-Hornet#11464
tgstation/tgstation#86510
tgstation/tgstation#83255
tgstation/tgstation#78225
tgstation/tgstation#78265
Fixes: #3530
Why It's Good For The Game
Harddels still bad, OpenDream good