-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Code Formatting of Repo - Add Preprocessing to Compilation Process - Introduction of Dev/Build Tools to WinUtil (Although very simple at the moment) #2383
Conversation
Nice! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good commits @og-mrk!
@ChrisTitusTech Fixed. Just discarded every change on my branch (HEAD) for every merge conflicts, and for |
e514b95
to
8ae328c
Compare
…irectory you call it from
…g Whitespace Characters
…ript - Make the workingdir same as sync.PSScriptRoot for consistency
…t-ChildItem' PS Cmdlet
Now a few things happened:
With that being said, I made the PR read to review. If you've anything you don't like or want to improve upon, @ChrisTitusTech you can do so. |
Could you rename the function and the file Do-preprossing to something following the function naming convention from Microsoft like Start-Preprocessing or Invoke-Preprocessing Seeing as there are not really any code conventions for winutil in place, and it's quite the wild west, I think it's probably smart to try to adhere to the one from Microsoft at least |
Yahh I do agree on this weird naming convention I've used 😅, also didn't check the Official Microsoft PowerShell Standards, I'll be sure to refer to it when adding Formatting Rules, thanks @Marterich |
…es - Update Documentation - Add Validations & Useful feedback upon error
…les' validation step check all filepaths before finally checking if any has failed
This is one big PR @og-mrk still reviewing it. Looks awesome from a cleanup and standardizing some of the files. |
Thanks @ChrisTitusTech for taking your time to review this. And although I'm happy with the result of this tiny script I've made (it's quite simple if you checked Therefore, I'll convert this PR to draft until I see/think it's ready (both "feature" and stability wise). I know the core concept & idea is finished (tested it with every change along the way).. but when you've RegEx which's untested to every possible case, and said RegEx can modify your entire code base.. yahh, I think I'll do some Unit Testing for this script. And the Tests themselves shouldn't change over time.. as long as the code itself (formatting rule) doesn't change, because the input (possible cases) for each Rule is many, but limited, and expected output is always one (the desired formatted code). |
Even though there's more Formatting rules I'd like to add.. the current (and somewhat simple) RegEx formatting is ok.. more complex formatting would require parsing PowerShell and figuring out the AST (Abstract Syntax Tree), which in the long run.. would make it a script that can be used in any project, and would work just as intended (formatting PowerShell code bases). @ChrisTitusTech Tested this script, and it works as intended. Will make PR ready for review... |
Type of Change
Description
Makes sure all files don't have any trailing whitespace.
And by putting in inside a function (the
Do-PreProcessing
function) it'll be possible in the future to do other things, like replacing Tabs with 4 spaces (the convention that's used the most.. other thanInvoke-WPFMicrowin.ps1
).Testing
I ran the script and not only does it compile WinUtil, but also makes sure every file follows specific coding conventions (like having no trailing whitespace)
Additional Information
Not only does it do this preprocessing on other files in the repo.. it also does it on the
Compile.ps1
file itself, indicating that PowerShell interpreter makes a Copy of the running script in memory, before actually running said script.. which's somewhat a new discovery (at least for me).Checklist