Skip to content

Conversation

ryanofsky
Copy link
Contributor

Reasons for rename:

  1. Default value is <datadir>/wallets so calling it -walletsdir instead of -walletdir would be more internally consistent.
  2. Directory can contain more than one wallet so plural makes sense
  3. This makes it harder to confuse -walletdir option with -wallet option if we store wallets in their own directories as proposed in Specify custom wallet directory with -walletdir param #11466 (comment) and implemented in scripted-diff: prefix [address|change]type parameters with 'default' #12216

Reasons for rename:

  1) Default value is `<datadir>/wallets` so calling it `-walletsdir` instead
     of `-walletdir` would be more internally consistent

  2) Directory can contain more than one wallet so plural makes more sense

  3) This makes it harder to confuse -walletdir option with -wallet option
     if we store wallets in their own directories as proposed in
     bitcoin#11466 (comment) and
     implemented in bitcoin#12216

-BEGIN VERIFY SCRIPT-
git grep -l walletdir | xargs sed -i s/walletdir/walletsdir/g
-END VERIFY SCRIPT-
@meshcollider
Copy link
Contributor

This was suggested on the original PR here:
#11466 (comment)

Both @laanwj and I agreed that walletdir is easier to remember and type :)

@laanwj
Copy link
Member

laanwj commented Jan 18, 2018

I strongly prefer the singular name. I'm going to forget the 's' every time. I think it's fairly clear that it's a directory that it can contain multiple things, but in any case that belongs in the documentation, I don't see a reason to rename it.

@jonasschnelli
Copy link
Contributor

Agree with @laanwj.

@ryanofsky
Copy link
Contributor Author

I did mention three reasons above (hopefully they make sense), and I don't exactly know when you would be typing this option, but if getting rid of the s here is important do you also want to make the default location <datadir>/wallet?

@ryanofsky
Copy link
Contributor Author

Ok, I don't have a strong preference. Maybe you had a bad experience with the letter s. Was mostly concerned about confusion between -walletdir and -wallet options if different wallets stopped being stored together in a single bdb environment. Will close.

@ryanofsky ryanofsky closed this Jan 18, 2018
@laanwj
Copy link
Member

laanwj commented Jan 18, 2018

Was mostly concerned about confusion between -walletdir and -wallet options if different wallets stopped being stored together in a single bdb environment. Will close.

Yes, that's a valid concern.

@Sjors
Copy link
Member

Sjors commented Jan 19, 2018

Fwiw, it's "computer store", not "computers store", "grocery bag", not "groceries bag", "cookie jar", not "cookies jar". Though I'm sure there's plenty of counter examples.

@promag
Copy link
Contributor

promag commented Sep 26, 2018

Another plural exception:

  QDesktopServices::DocumentsLocation
  QDesktopServices::FontsLocation
  QDesktopServices::ApplicationsLocation
* QDesktopServices::MusicLocation
  QDesktopServices::MoviesLocation
  QDesktopServices::PicturesLocation

so wallet is like music 🎶 ...

(not english native speaker, so maybe there is a reason to be singular in this case)

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants