winutil icon indicating copy to clipboard operation
winutil copied to clipboard

Remove Compiled Files, ignore Files and add Compile-Check on PR

Open Marterich opened this issue 1 year ago • 3 comments

Pull Request

Another try at resolving the "winutil.ps1 is included in a PR" once and for all and optimization to the Compilation / Merge process (Already updated to include code Signing, with a few fixes)

!! IMPORTANT !! This will most likely result in Merge Errors in the future. They can be resolved by manually deleting the winutil.ps1 and release.yaml from the PRs. This shouldn't be a problem for long. Just until the contributors pull the repo without winutil.ps1 or run git rm --cached .\winutil.ps1 to remove the file from their local tracking

Type of Change

  • [x] New feature

Description

  • Removes the winutil.ps1 from the Repo and the Git Tracking List
  • Add an Action to remove the winutil.ps1 automatically if it is pushed to any Branch. (The compiled winutil.ps1 from now on should only be stored in the Releases / Pre-Releases)
  • Adds a new Compile and Check action that runs on every PR, and tries a Compilation and a Syntax Check using "Get-Command -Syntax" and displays the result accordingly. (The Check is included in the Compile script to also benefit from the check when developing locally)
  • Modifies the Pre-Release Action to Compile the winutil.ps1 and add the CodeSigning Cert only on Chris's repo (because winutil.ps1 doesn't exist in the Repo anymore)
  • Removes the Full Release Action (unnecessary if I understand your workflow correctly. 1. Create Pre-Release, 2. Manually Edit Pre-Release to a Full Release)

Testing

I tested this on my fork but (of course) you should test yourself because this is quite a process change.

Impact

Cleaner PRs. No more 10000+ lines changed if someone deletes the winutil.ps1 More stable PR Merging Process, because the sanity check of compiling and checking the syntax is already run before the PR gets merged (potentially it's also possible to restrict the Repo from merging a PR if the Check fails)

Checklist

  • [x] My code adheres to the coding and style guidelines of the project.
  • [x] I have performed a self-review of my own code
  • [ ] My changes generate no errors/warnings/merge conflicts. <- :*D Most definitely not because deleting files is always fun...

Marterich avatar Jul 31 '24 16:07 Marterich

Do a PR to Contribute docs page stating how to remove the files.

Real-MullaC avatar Jul 31 '24 18:07 Real-MullaC

@Marterich, as @Real-MullaC have pointed in his comment, these changes require updating some documentation before accepting, as some already existing Local Clones would still track WinUtil (so guiding devs on how to "refresh" their Git Cache, and why do it in first place).

And Contribute Guide heavily encourage devs to use GitHub Desktop, which GitHub Desktop by default doesn't install Git CLI, which's needed to update the cache (I believe so, maybe I'm wrong on this part), so we'll need to write an "Optional" step which'll guide devs on installing Git CLI, and how to update their local Git Cache.

Also also (this's last thing 😅), there's a step that isn't require anymore (as winutil.ps1 will not be tracked by Git anymore), see screenshot below for more info.

Screenshots

image

og-mrk avatar Jul 31 '24 18:07 og-mrk

@Real-MullaC @og-mrk I actually have not worked with the GitHub Desktop App in ages, so I'll definitely fuck something up writing a Guide for how to remove the file there. For the Commandline it's as simple as

git rm --cached .\winutil.ps1
git commit -am "remove ignored files"

If it's not too much of a hassle for one of you, I would really appreciate it if you could create a PR containing the DOC changes to my Branch :) If not, I'll try to do my best later today or tomorrow...

Marterich avatar Jul 31 '24 19:07 Marterich

Couldn't find a way to merge all these changes, so I manually did all these changes and made some minor modifications where needed. The release script needed an env var for the code signing, but the rest was perfect! Thank you @Marterich

ChrisTitusTech avatar Aug 07 '24 20:08 ChrisTitusTech