Skip to content
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

Split of Scr & Lib files to 1 ps1 per function whether Private or Public #118

Closed
kilasuit opened this issue Jul 15, 2016 · 12 comments
Closed

Comments

@kilasuit
Copy link
Contributor

As a general practice it is more widely accepted that ps1 files should ideally only contain 1 function per file. This is good for ensuring that the code is easier to maintain although this would be a big piece of work to get right with the module.

I'm looking at trying to do this and there are some issues in doing so cleanly due to the below in the Src files

  • LabNode.ps1, LabHostDefault.ps1, LabConfiguration.ps1, LabResource.ps1,LabHostConfiguration.ps1, LabVM.ps1, LabMedia.ps1, LabVMDefault.ps1 contains private & public functions
  • LabVMDisk.ps1, LabVMDiskFile.ps1, LabVMSnapshot.ps1, LabSwitch.ps1, contains only Private Functions
  • Lab.ps1 contains only Public Functions

All of the files in Lib contain only Private Functions but again for consistency I would be more suggestive of separating the functions within them for maintainability reasons going forward

@iainbrighton
Copy link
Contributor

@kilasuit I have no qualms with this.

Are you volunteering?! If so you might want to wait for #106 to get merged in as there's a whole load of updates 😛

@kilasuit
Copy link
Contributor Author

Yeah sure will try and get this prepped this weekend as it shouldn't actually be too much work to do :-)

@kilasuit
Copy link
Contributor Author

Actually may have this done quicker than I thought with a blog post to come out of it too!

@iainbrighton
Copy link
Contributor

iainbrighton commented Jul 15, 2016 via email

@kilasuit
Copy link
Contributor Author

The export of all functions was easily enough done - However I'm thinking that once #106 gets merged in that would be a good starting point to then work on the refactor cleanup & build out some the non-functional tests organised too for the 0.9.12 release with this additional rework.

I'm trying to work out where the right level (as in number) of tests would be for this module so I'll be doing some testing on a few of my own modules as well that are significantly smaller so that I can more accurately judge going forward but lets say that thins point I have a significant number of tests Auto-Written and all the work behind this will go into the PesterHelpers Module too so could be reused in other modules too.

But I've got it all working for testing Public & Private Functions too with a few little kinks to iron out but its almost where I'm happy with it

@iainbrighton
Copy link
Contributor

I'm thinking that the next release will be v0.10.0 or we could push for v1.0.0? That will mean documentation being complete too /cc @it-praktyk. Either way, we need to make sure all 3 big changes (#4, #106 and #117) can be merged with minimal merge conflicts.

@kilasuit
Copy link
Contributor Author

I was thinking perhaps along the lines of 0.9.12 should be just #106 merged in & then 0.9.13 can be the split out and documentation as that's pretty much non-functional changes coming together and then bump to 0.10.0 at that point??

@iainbrighton
Copy link
Contributor

iainbrighton commented Jul 18, 2016

IMHO - as #106 is a fairly big change, this will need some serious testing before it gets merged in.. I'm not sure how many people will be testing this?

@iainbrighton
Copy link
Contributor

@kilasuit Have you got working code to be able to split these files? It would be handy to do this before performing a code review (I think we're getting close to a v1.0 release)..

@kilasuit
Copy link
Contributor Author

Yeah - Will do a pull of the latest dev branch and work on this in my fork.

@iainbrighton
Copy link
Contributor

@kilasuit Thank you! I'm thinking about putting the public functions in the \Src\Public folder and all private functions in the Src\Private folder (and drop the Lib folder altogether)? This will no doubt have a big impact on the tests, but I can sort them out if necessary.

kilasuit added a commit to kilasuit/Lability that referenced this issue Oct 23, 2016
iainbrighton pushed a commit that referenced this issue Oct 25, 2016
* path changes for Private & Public functions
* Split out Scr/Lib to Private/Public
* fixes #118
@iainbrighton
Copy link
Contributor

I think this is finally done. Phew 😅!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants