dogecoin / dogecoin

very currency
MIT License
14.4k stars 2.8k forks source link

doc: removed dead links #3513

Open daank-c opened 1 month ago

daank-c commented 1 month ago

This removes 3 dead links from the shared-libraries.md document and adds some missing punctuation.

patricklodder commented 1 month ago

Although this is cool, I'm not sure if this documentation is correct. Looks like it was never ported, just someone doing sed s/bitcoin/dogecoin/g on it. I'll go figure out what is actually real in this and what isn't if you don't mind.

Re: punctuation. Note that none of the list items are sentences, so normally those don't get punctuation?

daank-c commented 1 month ago

I'll go figure out what is actually real in this and what isn't if you don't mind.

I don't mind at all.

Note that none of the list items are sentences, so normally those don't get punctuation.

The 'Parameters' list above the 'Script Flags' and 'Errors' lists uses punctuation, so I made the punctuation consistent across all lists. I can restore the punctuation back to how it was, if that's preferred.

patricklodder commented 1 month ago

I can restore the punctuation back to how it was, if that's preferred.

Let's see what in those lists is actually real documentation for this repo first.

patricklodder commented 1 month ago

So, this document does not reflect implementation, at all. The replacement of bitcoin with dogecoin has not been reflected at all in the actual header file. I do think that the symbol names reflected in the documentation are desirable over the symbols actually exported, especially because these would conflict with someone importing both libbitcoinconsensus and libdogecoinconsensus 😅

So as of right now, I would not trust anything this document says. I see 2 options:

  1. Fix the library to actually use correct symbols (and make sure it actually works)
  2. Deprecate the whole library and the docs. I do not know anyone that currently uses it - it's not really useful anyway.
daank-c commented 1 month ago

I would be fine with deprecating the whole library and documentation if it's not being used.

Looks like bitcoin is doing the same bitcoin/bitcoin#29189.

patricklodder commented 1 month ago

Looks like bitcoin is doing the same

Bitcoin Core is replacing this library with another, more complete library, libbitcoinkernel.

daank-c commented 1 month ago

Do we have any plans to do the libbitcoinkernel thing to Dogecoin in the future? (seems like a lot of work)

patricklodder commented 1 month ago

Do we have any plans to do the libbitcoinkernel thing to Dogecoin in the future? (seems like a lot of work)

Personally, I think that having a well maintained shared library that exposes this codebase could help when testing alternative implementations and therefore would expect such a request from serious alternatives and auditors. But, having a non-maintained library like the one we have now (where for almost 5 years its docs could not have been more misleading, because every symbol is misnamed) is however a waste of everyone's time and effort.

So unless someone is going to use it, I'd not prioritize that.

patricklodder commented 1 month ago

I'm going to change this to draft and take it off the review board for now, because whatever we do, this change on its own doesn't make sense - i.e. will need to do work either way. However, since we already started the discussion here, we might as well have the discussion here, and give people some time to think about their wants/needs wrt this, and react.

daank-c commented 1 month ago

Sounds good, and I agree that it needs work either way.

Just one note: if a solution isn't found before the next release, it would make me more comfortable if there was at least a label at the top of the document saying "this library is not actively maintained" or something to make it obvious that it's not useful at the moment. I just wouldn't want to see anyone waste their time or become confused by reading it.

patricklodder commented 1 month ago

Documented a reminder in #3515