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

Create formal, public and documented module #2733

Closed
Ash258 opened this issue Nov 4, 2018 · 5 comments
Closed

Create formal, public and documented module #2733

Ash258 opened this issue Nov 4, 2018 · 5 comments

Comments

@Ash258
Copy link
Contributor

Ash258 commented Nov 4, 2018

While writing manifests, you could use some scoop's internal functions inside install scripts (pre_install, post_install, script), but they are not documented, use poor parameters, which are booleans (not switches), and you need to go to github, search for name, through it's scoop's code almost everytime to look for parameters, which then look like extract_7zip $file $path $false instead of more intuitive way Scoop-Extract7zip $file $path -Recurse (with positional parameters, otherwise, some -Archive $file -Destination $path -Recurse).

There should be some wrapper module, which would be properly documented (wiki, and ps1 files). All of that functions will only be oneliners with param block and some defaults

or

rewrite scoop's files to match single style (It's code is pretty mess with naming).

Some functions does not return values from functions, some have. Newly added functions are Pascal-Case named, while od ones are named using snake_case.
Pascal case in lots of snake_cases, Function with return, Function without return

Example:

<#
.SYNOPSIS
SOME DESCRIPTION
...
#>
Scoop-Extract7zip {
    param(
        [Parameter(ValueFromPipeline..., Mandatory, ...)]
        [ValidateScript(...)]
        [String] $Archive = $fname
        [Parameter...]
        [ValidateScript(...)]
        [String] $Destination = $dir
        [Switch] Recurse
    )

    process {
        extract_7zip $Archive $Destination $Recurse
    }
}

Scoop-ExtractMsi {...}

Also.
Why is msi block Deprecated? Sure, you can extract MSI, but there are some applications, which needs to be installed (initializing some drivers, services, ...) and marking it as deprecated is wrong. It's forcing maintainers to think that support of msiexec will be removed and manifest will be broken. And intead of using supported json properties file, args you need to write script block with custom call to msiexec.

@r15ch13
Copy link
Member

r15ch13 commented Nov 4, 2018

Refactoring Scoop to use Pascal case in general would be better. Since it's the preferred style for PowerShell. This will be a lot of work, but some changes that have to be added, exception handling and proper state handling for failed app installations, require a complete rewrite anyway.

Adding wrappers might be useful for writing tests and transition to the refactored code.

@r15ch13
Copy link
Member

r15ch13 commented Feb 24, 2019

Regarding the msi property, do you have an example where extracting it isn't enough?

@Ash258
Copy link
Contributor Author

Ash258 commented Feb 24, 2019

I was just curious. But for example. iCue. it's Corsair's utitlity for peripherals, which is need to be installed due to some services, hooks and registry configurations for proper functionality.

@linsui
Copy link
Contributor

linsui commented Sep 1, 2019

@r15ch13 Here is an example. wireguard must be installed because it needs to install wintun.

@Ash258 Ash258 closed this as completed Jul 8, 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

No branches or pull requests

3 participants