Open lipkau opened 9 years ago
"1 line code blocks” agree there are exceptions where it just does not make sense. "Documentation Comments relative to function” Not touching this one, everyone has their opinion, I would say all are valid and just leave it at that. "How to make param mandatory” Kind of agree on this one but feel if you need mandatory params just make it and advanced function and use the parameter properties to make it mandatory. "-verbose / -debug” agree and would also add -help to the list, see the -help a lot. “[Alias()]” Agree on this one, it shows lack of knowledge on how PS works with parameters when I see it "Unit Tests” No clue on this one, still learning about it. "Module Folder Structure” hard one outside of pester tests. I use Assemblies for DLLs I import on mine. the rest just put in folder to organize per general function. "How to build a proper .psm1 file” I would prefer to specify each one rather that listing and adding them dynamically. Starting to see backdoor profile files in capture the flag events and used by pentesters, this just adds an easier way for code to be added by accident.
Personally, I don't think brace placement or comment-based help block placement matters, so long as it's consistent across the codebase. When you contribute to a project, if they've specified a style, then adhere to that style. (If it's already an inconsistent mess, then just use your best judgement, I suppose.)
My own personal preference is to put short conditionals all on one line. You'll often see this sort of thing in a finally
block for cleanup, in my code:
finally
{
if ($session) { Remove-CimSession $session }
}
@dlwyatt +1. I even do the same in C#
if (arg1 == null) throw new ArgumentNullException(nameof(arg1));
I do this sometimes... particularly when pasting code into a script from a command-line history.
In C# I insist that if you want to leave braces off, it must go on one line, like what @rkeithhill wrote.
Despite that -- it's absolutely not a best practice. That is, I would be mildly opposed to having an automated check that stopped people from doing it occasionally, but I would also be opposed to even mentioning that format in a style guide, and I'd never intentionally put code like that in a lesson plan or presentation.
Long experience has shown that the way that leads to the least problems in the future (in every language) is to always put the closing brace on it's own line, and never put code inside the brace on the same line as the opening brace. It's just a lot harder to screw up when you come back to add something later.
I have tried over the years every place where PowerShell allows the doc comments: above (like I was used to in C#) or at the foot (just to get it out of the way), and inside the top ... my recommendation is what is in the guide (I think I wrote this paragraph, and I just added a headline to it so you can't miss it). Basically:
This is purely about the "pit of success" and is in no way meant to imply that putting them elsewhere is wrong -- I just believe that you'll make less mistakes in maintenance this way.
Your comment is dead on. Code that assigns a default value which throws is legacy code from before there was a [Parameter()]
attribute in PowerShell. Feel free to provide that as a counter example (and mention that people may see it a lot, but only because there's a lot of code around the internet from PowerShell 1.0 still).
Unless you want to have a go at writing those sections, I think it would be worth filing separate "issues" for each of these so that people can pick them up and write them. Your comments are dead on.
nuget
for binaries, or git submodule
for source code, and provide a setup and/or build script for people to generate packages.We've been playing with some models for folder structure and build/test/publish scripts in our experimental development processes project (PoshCode\PSGit) -- I don't think we're done playing, but it has both binary and source dependencies, and an automated build with 100% code coverage, so it's worth looking at as a possible example of how to structure things ;-)
Personally ....
I am mildly opposed to having many script files for modules.
I strongly recommend you avoid using gci to import all the scripts in a subfolder ever. You're creating a code-injection vulnerability, and in this case, also breaking PowerShell's ability to guess what commands are in your module if you used ExportedFunctions = "*" (which you ought to avoid, but nevermind that).
If you must have nested files, make them psm1 files, and always list each of them explicitly. This avoids merging the variable scopes of all those scripts together, and protects you from silly bugs where someone editing one file is unaware of variables/functions in another file.
Keep functions which depend on each other in the same module, and only break out modules of functions which are self-contained (it's ok to depend on a sub module in your main module, but not OK for sub-modules to depend on each other).
Hi.
Note: I really looked into the exisiting tickets before posting, but I might have overlooked something and included something you already discussed
I am really exited about this repo and already decided to change some stuff I do when coding. While thinking about how I code stuff I thought of these situations and am looking forward to your opinion on them:
Altought an article arlready suggests having
{
in a dedicated line, I do get annoyed with block like:I write it like:
The section DOC-01 Write comment-based help already ilustrates this, but I think a section to "where to put the comment" would be nice.
vs
vs
I don't like it, that
Get-Help throw
returnsMaybe spend a
# BAD
example on this in When using advanced functions or scripts with CmdletBinding attribute avoid validating parameters in the body of the script when possible and use parameter validation attributes instead.?Best Practice on what kind of information should be return in
Write-Verbose
andWrite-Debug
?Any best practices for
[Alias()]
? I don't like code where an alias is defined just to shorten a parameter nameAny recommendations on this? Shouldn't a .\Tests\ folder make sense in a Module?
I see no use of making a single of .ps1 file for each function in a module with 2 - 3 functions. But where is the limit? Any recommendations in the structure?
Any best practice on how to load .ps1 files in a module?
I currently use