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

Rename overrideSettings to just settings #83

Open
slifty opened this issue Aug 26, 2020 · 2 comments
Open

Rename overrideSettings to just settings #83

slifty opened this issue Aug 26, 2020 · 2 comments

Comments

@slifty
Copy link
Member

slifty commented Aug 26, 2020

Task

Description

IAppliance has a documented constructor parameter of overrideSettings

As @chriszs points out in this PR, really those are just settngs, what is override doing there?

Unless someone can articulate a good reason for calling it overrideSettings, lets change it to settings.

Note: this doesn't actually impact the API, so this would be a patch level fix.

Relevant Resources / Research

None.

Related Issues

None.

@reefdog
Copy link
Contributor

reefdog commented Aug 30, 2020

I wrote this up more forcefully and elegantly already but can’t find where.

Have you considered the name options over settings? I think of settings as initial system configuration, e.g. environment configuration, whereas options seems a more appropriate name for function or instance parameters.

@reefdog
Copy link
Contributor

reefdog commented Aug 30, 2020

(I do agree with getting rid of the override prefix, mostly because it always looks misspelled to me. Kind of like “misspelled”.)

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