microsoftgraph / microsoftgraph/msgraph-sdk-powershell

Exactly what are you trying to do in your auto-generated modules?

Abierto
#372 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

  • #766 de @georgend — cerrado sin fusionar
AutoREST-dependency
Lenguaje dominante
C#
Estrellas
898
Forks
230
Merge medio
2 d 5 h
PR fusionados (30 d)
31

Descripción

Discover of issue #371 led to me digging into your module internals more, and I think you need to re-think what you are doing with your auto-generated modules.

Let's take Microsoft.Graph.Users.User version 0.9.1 as an example.

It contains a top-level module that is defined using a psd1 file, a dll, and multiple psm1 files.

The psd1 file identifies the psm1 file as the root module, but it does not identify the dll as a nested module to load as part of the NestedModules entry in the manifest. Instead it leaves the psm1 file to manually load the dll.

Issue: There is no need to manually load a nested binary module like this.
Recommendation: Simply add the relative path (e.g. './bin/Microsoft.Graph.Users.User.private.dll') to the NestedModules entry in the manifest, and PowerShell will load it for you. That's what NestedModules is for!

Once the binary module is manually loaded, the main psm1 file loads a custom psm1 script module that in turn loads an internal psm1 script module. The custom psm1 script module then goes on to disable implicit exports, and then dot source any nested ps1 files that are in the custom folder.

Issue: You don't have any nested ps1 files in custom folders in any of the Graph modules. None.
Recommendation: Maybe this is some legacy code? It's just more processing for nothing though, so if it's legacy, remove that logic. If not, comment it out until you need it. No need to do file I/O where it is not required.

The custom psm1 file then goes on to export the functions and aliases that are defined at that point, which will make them visible to the parent module.

The internal psm1 file switches the contents of a subfolder based on the Graph profile that was selected, and then reads all ps1 files in that profile subfolder to get the command definitions for that module. It then exports the functions and aliases, which will make them visible to the parent module.

Going back to the top-level psm1 file, it also reads the profile settings and loads the cmdlets from the exports path.

Issue: Why does it take three psm1 files to do what one psm1 file could do for you? There doesn't seem to be any need for your custom psm1 file, nor does there seem to be any need for your internal psm1 file. Your top-level psm1 file can do (and is already doing!) what is needed without the others adding extra work to the loading of these modules. Sure enough, if I look at the information stream output, I see multiple duplicate messages that reinforce that there is unnecessary duplication being done here.
Recommendation: Reel this back in to just one psm1 file, one psd1 file, and one dll, to keep things performant and eliminate duplication of effort. You don't need more than a single psm1 file with what you have today. Auto-generated code can and should be clean.

Other miscellaneous issues:

  • You're using pipelines in your psm1 files where they are not necessary. Pipelines add overhead to processing times. They should be used when processing large amounts of data that you don't have in memory. They should not be used when you have data already in memory. Examples taken from Microsoft.Graph.Users.User.psm1:

    Instead of this Do this Reason
    $directories = Get-ChildItem -Directory -Path $exportsPath $directories = @(Get-ChildItem -Directory -Path $exportsPath)` This is necessary for the changes recommended below
    if(($directories | ForEach-Object { $_.Name }) -contains $instance.ProfileName) { if($directories.foreach('Name') -contains $instance.ProfileName) { Collection is already in memory (performance), code is easier to maintain (simplicity)
    $profileDirectory = $directories | Where-Object { $_.Name -eq $instance.ProfileName } $profileDirectory = $directories.where{$_.Name -eq $instance.ProfileName} Performance and simplicity, as above
    } elseif(($directories | Measure-Object).Count -gt 0) { } elseif ($directories.Count -gt 0) { No pipeline (performance) and and simplicity again
    $profileDirectory = $directories | Select-Object -Last 1 $profileDirectory = $directories[-1] No pipeline (performance) and simplicity again
    Get-ChildItem -Path $exportsPath -Recurse -Include '*.ps1' -File | ForEach-Object { . $_.FullName } `foreach ($file in Get-ChildItem -Path $exportsPath -Recurse -Include '*.ps1' -File) {. $_.FullName } Collection is small, faster to process using foreach keyword (performance)

Last, in your psd1 file you are still using wildcards for AliasesToExport. When a single module does hit, it results in a performance hit for all of PowerShell because command discovery is affected. Please, please replace AliasesToExport with the actual list of aliases that are exported, or an empty array if there are none. Ditto for FunctionsToExport and CmdletsToExport.
AB#7392

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza con los archivos generados Microsoft.Graph.Users.User.psd1 y Microsoft.Graph.Users.User.psm1; después, inspecciona los archivos psm1 personalizados e internos descritos en el issue. Compara la lógica de carga de módulos, exportación, selección de perfiles y procesamiento de pipelines con las recomendaciones. Se considera terminado cuando los módulos generados usan la estructura más sencilla de manifiesto y carga, evitan el procesamiento innecesario y enumeran exportaciones explícitas.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
powershell
Área
tooling
Tipo de issue
Refactorización
Dificultad
5/5
Tiempo estimado
Más de una semana
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
25/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.