Add autoload for TestLanguage.psm1 TestHelpers.psm1 - #3456
Conversation
Jason Shirk (lzybkr)
left a comment
There was a problem hiding this comment.
2 small comments
- you can delete most of the comments from the module manifest, they aren't useful
- can you think of a better name for the module
TestLanguage- it feels too similar to a hypothetical script or cmdlet. MaybeLanguageTestHelpers- and maybeTestHelpersshould beCommonTestHelpers, or something like that.
|
Can we add a "Module" suffix?
|
|
Mostly I'm just thinking out loud here, it just struct me as odd seeing verbs in module names. I was curious how common this was, and it's not uncommon: $verbs = (Get-Verb).Verb
Find-Module | ? { $modName = $_.Name; $verbs | ? { $modName.StartsWith($_) } }This gives my 97 modules, some of which sound more like scripts than modules, but it's hard to see without looking more closely. At any rate, consider this just one opinion, nothing more. |
|
I thought about how to easily find these modules using
|
e040758 to
a138e0d
Compare
|
Jason Shirk (@lzybkr) Travis Plunk (@TravisEz13) In last commit I renamed modules (Removed approved verbs (Get-Verb) from module names). |
|
Jason Shirk (@lzybkr) Travis Plunk (@TravisEz13) Do I need something else to do here? |
5a3e8fd to
e8d7619
Compare
|
Jason Shirk (@lzybkr) Travis Plunk (@TravisEz13) Could you please review and merge? This blocks further work. |
There was a problem hiding this comment.
There are still comments here
There was a problem hiding this comment.
can be removed
There was a problem hiding this comment.
see previous comment
There was a problem hiding this comment.
see previous comment
Travis Plunk (TravisEz13)
left a comment
There was a problem hiding this comment.
I don't see any major issues.
|
Travis Plunk (@TravisEz13) Could you please continue with the PR? |
|
Ilya (@iSazonov) Sorry, I've been out of the office for a few weeks. |
|
Closed and re-opened PR to re-trigger CI. Could you rebase as well? |
Test.Helpers.psm1 was renamed to TestHelpers.psm1
Remove approved verbs (Get-Verb) from module names.
eccc5c3 to
4c7b812
Compare
|
Rebase done. |
|
Travis Plunk (@TravisEz13) Is the PR ready to merge? |
|
Yeah Ilya (@iSazonov), Thanks. I was giving people time to give feedback. Thanks for pinging me too. |
* Add autoload for TestHelpers.psm1 Test.Helpers.psm1 was renamed to TestHelpers.psm1 * Resolve conflit and rebase Add autoload for TestLanguage.psm1 * Remove unneeded comments from PSD1 files * Rename test modules Remove approved verbs (Get-Verb) from module names. * Enhance ShouldBeErrorId to output exception into pipeline for later analysis * Remove unneeded comments * Resolve merge conflict
The test temporary modules moved to
test\tools\Modules.I did two commits to simplify the review: