Skip to content

Move classes from PowerShell to C##350

Merged
DarqueWarrior merged 192 commits intomasterfrom
moveclasses
Sep 23, 2020
Merged

Move classes from PowerShell to C##350
DarqueWarrior merged 192 commits intomasterfrom
moveclasses

Conversation

@DarqueWarrior
Copy link
Collaborator

@DarqueWarrior DarqueWarrior commented Aug 9, 2020

PR Summary

This is a major architecture update. All the classes have been moved from PowerShell to a C# class library.

This changed the build because it has to build both a .NET Core class lib and the PowerShell parts.

I also added a few new functions to keep the Cache code consistent.

I also reviewed every function to make sure the code is consistent across the module.

PR Checklist

# area : Build
# resourceName : Definitions
# routeTemplate : {project}/_apis/build/{resource}/{definitionId}
# https://bit.ly/Add-VSTeamBuildDefinition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this witchcraft? What did you do there or how? That is cool. Did you add every cmdlet here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I reviewed each function I found the docs page and created the short link. I don't have one for each yet but I will continue to create as I review the functions.

@SebastianSchuetze
Copy link
Contributor

Is this change also making the module cross-platform ready or not? There was one dependency I cannot remember anymore that was making it not.

Copy link
Contributor

@SebastianSchuetze SebastianSchuetze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed most of the files to understand it myself. Many comments are learning questions for me. If you could answer them, then it would help my understanding of this PR.

But I really like the C# part for the more inner workings of the classes.

I haven't reviewed the unit tests as I feel I don't understand enough without running the unit tests with the core library myself.

this.Prime(list);
}

internal void Prime(IEnumerable<string> list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it for? Why is it named "Prime"?
Update: I can see that it is clearing a cache variable and repopulating it, but I don't know what the wording has to do with the business logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for priming the cache with a list. For example, if you call Get-VSTeamProject you can prime the cache with the projects you just got back. You prime pumps by filling them with water before you use them. We are just priming the cache before we use it.

But we can change it. What word makes the purpose more clear?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something more direct like PreFill or Initialize?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change to PreFill

$latestMod = Find-Module -Name VSTeam

if($latestMod.Version -ge [vsteam_lib.Versions]::ModuleVersion) {
Write-Host "There is a new version of VSTeam available ($($latestMod.Version)). Run 'Update-Module -Name VSTeam' to update."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe in color yellow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am having second thougths on this because it really adds a lot of time to the module import time. Hey, @smurawski is there a better way for a module to check for a newer version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It goes from instant to a very noticeable amount of time.


function Add-VSTeam {
[CmdletBinding()]
[CmdletBinding(HelpUri='https://methodsandpractices.github.io/vsteam-docs/modules/vsteam/Add-VSTeam')]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DarqueWarrior I hope you don't kill me, but due to technical improvements I had to change the basic url from

https://methodsandpractices.github.io/vsteam-docs/modules/vsteam/
to
https://methodsandpractices.github.io/vsteam-docs/docs/modules/vsteam/

there is the "docs" path being added. Nothing will change in the future. not sure if I can get rid of it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem that is an easy change with regular expressions. To confirm I just need to add 'docs' between 'vsteam-docs/' and '/modules/". Is that correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes confirmed!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will a new version of the site be pushed with the path change? I just noticed the new paths are 404s right now. I would like this to be updated again before I merge the latest changes into master.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, @SebastianSchuetze any ETA on when a new version of the docs site will be pushed so the new URLs like this one works?
https://methodsandpractices.github.io/vsteam-docs/docs/modules/vsteam/Add-VSTeam

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can push it today in 1 hour. Just tell me soon after if we keep one module or split in two. I prepared sub doc pages for vsteam plus, but it is basically just menu and placeholder intro pages.

@SebastianSchuetze
Copy link
Contributor

@DarqueWarrior site is now pushed:
https://methodsandpractices.github.io/vsteam-docs/

maybe you want to add this URL to the VSTeam repo metadata so users can directly access it from the repo homepage.

@DarqueWarrior
Copy link
Collaborator Author

@DarqueWarrior site is now pushed:
https://methodsandpractices.github.io/vsteam-docs/

maybe you want to add this URL to the VSTeam repo metadata so users can directly access it from the repo homepage.

The links still don't work.
site = https://methodsandpractices.github.io/vsteam-docs/docs/modules/vsteam/commands/Add-VSTeam
functions = https://methodsandpractices.github.io/vsteam-docs/docs/modules/vsteam/Add-VSTeam

There is a new commands part. I can change all the links. I just need to know which is going to be the final URLs.

@DarqueWarrior DarqueWarrior merged commit ace10a6 into master Sep 23, 2020
@DarqueWarrior DarqueWarrior deleted the moveclasses branch September 23, 2020 06:07
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.

3 participants