search

mimblewimble / grin-wallet

Grin Wallet
Apache License 2.0
185 stars 134 forks source link

Outdated code and docs #580

Open DavidBurkett opened 3 years ago

DavidBurkett commented 3 years ago

"Method" is no longer supported to the best of my knowledge. https://github.com/mimblewimble/grin-wallet/blob/429db61bb7ef42f009abb8886fd90221301dfc74/src/bin/grin-wallet.yml#L59-L67

Keybase adapter still exists and should be removed: https://github.com/mimblewimble/grin-wallet/blob/7f5c010fed2f9177e0e24cc3eaa7b79be112390f/impls/src/adapters/keybase.rs

Wiki doesn't cover slatepacks: https://github.com/mimblewimble/docs/wiki/How-to-use-the-Grin-wallet

cliik commented 2 years ago

I can't speak to the first two items, but I also have a question about the third:

Slatepacks are covered in docs.grin.mw/wiki, but this makes me wonder why we have two competing wiki's with different information. Should we consider obsoleting/removing the GitHub wiki in favor of the new docs.grin.mw/wiki? I'm willing to volunteer some time to help that migration, but I don't have enough context to decide what should stay or go.

phyro commented 2 years ago

David is right. The method has been deprecated and removed from ListenArgs. I think we can safely remove all the args in the grin-wallet/src/bin/grin-wallet.yml because we never parse these as can be seen in the listen implementation below the ListenArgs. This seems unused and can be removed now. I'll open a PR in a few days if nobody else beats me to it - and you're welcome to do so.

@cliik the wiki David linked is a "for the user" tutorial which is outdated. I completely agree we don't need/want to have two separate documentations on the same thing because it only introduces problems with synchronisation which eventually leads to them becoming out of sync (as is the state today). I do wonder how much we can throw away from the github wiki. I've checked two random entries Run a node on Google Cloud for free and Build & run automation and they both seem to be on the docs page here and here in the docs repo. I'm not too familiar with the full content of the two either. I'd agree the duplicate entries should definitely go away.

cliik commented 2 years ago

I'll open a PR in a few days if nobody else beats me to it - and you're welcome to do so.

I'll leave it to you. You're more familiar with that part of the codebase than me :smile:

I'd agree the duplicate entries should definitely go away.

I'd go a step further and say the entire duplicate wiki should go away (not just duplicate entries), to prevent these sort of inconsistencies from arising again. Of course migrating wikis is no small effort, so I don't blame anyone for the current situation, but if we are going to address issues like this, it might be best to address the root cause by wholesale migrating the old wiki to the new location.

I'm not sure who would need to approve such a move (if it even would get approved), but if so, I am willing to volunteer some time to the effort. Clearly someone put a lot of thought into the old and new wikis, so probably best to consult those people. Do you know who to ask about this?

phyro commented 2 years ago

I very much agree the optimal is to have a single reliable source in this case. Perhaps one thing to note is that maybe there are links to wiki page from articles in 2019 or similar e.g. to page Grin for bitcoiners or something. I'd say the best place to ask is to ask the community on the forum. Someone that made great work on the documentation goes under Paouky on the forum. He hasn't been around for some time though.