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

Deleting SCOOP_BRANCH not switch to master #3401

Closed
Ash258 opened this issue Apr 28, 2019 · 2 comments
Closed

Deleting SCOOP_BRANCH not switch to master #3401

Ash258 opened this issue Apr 28, 2019 · 2 comments

Comments

@Ash258
Copy link
Contributor

Ash258 commented Apr 28, 2019

Extracted from #3397

Well apparently deleting the SCOOP_BRANCH config variable did not reset the branch to master correctly. Can't seem to reproduce the issue.

Since SCOOP_BRANCH config is default and created always on installation, it is required for functioning. Since we can't forbid users to delete it we need to adjust it in code.

Should be enough to replace get_config SCOOP_BRANCH with get_config SCOOP_BRANCH 'master' to return default master, when config value do not exists.

@Ash258
Copy link
Contributor Author

Ash258 commented Apr 28, 2019

We should create some Hashtable with defaults which would be used in codebase.

$CONFIG_DEFAULT = @{
    'SCOOP_BRANCH' = 'master'
    '...LESS_MSI' = $false
    'aria2' = @{
        'split' = 5
    }
}

So we do not need to duplicate code and could easily change it if needed.

Also get_config could return defaults withotu providing it.

So instead of get_config 'SCOOP_BRANCH', which will be changed to get_config 'SCOOP_BRANCH' 'master', you could implement by default get_config 'SCOOP_BRANCH', which would return $CONFIG_DEFAULT['SCOOP_BRANCH'] if none was found.

https://github.com/lukesampson/scoop/blob/aa8513fb3fb7a3ae6311ec5c6333b2f0a6a8dc2e/lib/config.ps1#L44-L49

L46: return $CONFIG_DEFAULT[$name]

@Ash258 Ash258 changed the title Deleting SCOOP_BRANCH to not switch to master Deleting SCOOP_BRANCH not switch to master Apr 28, 2019
@niheaven
Copy link
Member

I like the idea. See discussion before: #3242 (comment)

Ash258 added a commit to Ash258/Scoop-Core that referenced this issue Dec 4, 2019
- Closes ScoopInstaller#3770

Configuration name is subject of change. I am following dash notation for naming, until config is reworked and united (Relateed ScoopInstaller#3401)

Is boolean enough or it is prefered to go with `scoop config 'default-architecture' '32bit|64bit'` which could be more suitable for additional arch support (arm for example)

![A](https://i.imgur.com/RBzEysd.png)
@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

2 participants