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

Simplify merging of files #265

Merged
merged 2 commits into from
Mar 11, 2020
Merged

Simplify merging of files #265

merged 2 commits into from
Mar 11, 2020

Conversation

MichelZ
Copy link
Contributor

@MichelZ MichelZ commented Feb 25, 2020

PR Summary

Simplify merging of files

Source/Classes/_classes.json Outdated Show resolved Hide resolved
@DarqueWarrior
Copy link
Collaborator

DarqueWarrior commented Feb 25, 2020

The reason the _classes.json exist is because the classes had to be loaded in a particular order. If you notice the files are not loaded in the order they would be listed in a *.ps1 search. The formats.json I think it fine this way but I don't believe the module will load correctly with classes this way.

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 25, 2020

@DarqueWarrior Yes, I know.
This is why I added a feature to be able to list the base classes explicitly, but load everything else using *.ps1. As you can see, the unit tests still work :)

@DarqueWarrior
Copy link
Collaborator

@DarqueWarrior Yes, I know.
This is why I added a feature to be able to list the base classes explicitly, but load everything else using *.ps1. As you can see, the unit tests still work :)

The newer unit tests load the needed classes explicitly.

$here = Split-Path -Parent $MyInvocation.MyCommand.Path
$sut = (Split-Path -Leaf $MyInvocation.MyCommand.Path).Replace(".Tests.", ".")

. "$here/../../Source/Classes/VSTeamVersions.ps1"
. "$here/../../Source/Classes/VSTeamProjectCache.ps1"
. "$here/../../Source/Private/common.ps1"
. "$here/../../Source/Public/$sut"

Those tests are not testing the module loading. The classes that are needed are loaded in the correct order for the tests. If those classes are loaded in the wrong order everything will fail.

@DarqueWarrior
Copy link
Collaborator

It looks like the code can be used for some files just not the classes.

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 25, 2020

@DarqueWarrior hmmm... ok, I'll have a closer look then and eventually back out the classes.
Thanks for the hints

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 26, 2020

@DarqueWarrior I've removed the classes changes

@DarqueWarrior DarqueWarrior merged commit 955bce0 into MethodsAndPractices:master Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants