Best practices, initial version

[sergey-shandar]

This is an initial version. I plan to add more best practices, especially related to coding.

Should we split the document into multiple, such as "generic", "Rust", "Git"? If yes, I can do it in the next PR.

Here's a preview of the document.

[wileyj]

It would be helpful if we can create an example repo that follows the rules you're suggesting here.
Otherwise, some minor suggestions on my end but overall looks great, thanks!

[AshtonStephens]

I really like and agree with the principles in this document. I like what you're saying about I/O as well as the way this document suggests we should organize our branches when supporting bugfixes for multiple release versions. I'm also a huge fan of the git graphs and the plethora of links you included in it, its very easy to follow (though the link to Occam's razor may have been overkill) :)

I have a few questions and suggestions:

1. How do we expect to use this document and what is the tangible outcome we'd like to see?

To me, this document is missing a clear purpose / call to action. For example, the CI tenets document was originally intended to justify a sequence of CI changes that I was going to make to Trust Machines' CI pipelines and then further justify tickets in the stacks-network ecosystem. That didn't happen because I got pulled in another direction but if someone wanted to start improving some CIs then the document would be a great tangible starting point to justify cutting tickets, especially since it had consensus agreement from Trust Machines' core devs when it was published. I'm curious what tangible action we're expecting to make as a result of this document or how we intended to enforce the standards.

Please don't take this to mean "I think we shouldn't have a document that outlines these things." Instead, I'm curious what we hope to happen when it's released and want to be strategic in how and where to release it to best fit that goal.

2. It would be nice to split these standards into separate documents.

Should we split the document into multiple, such as "generic", "Rust", "Git"? If yes, I can do it in the next PR.

In my opinion, yes. The document in this PR outlines git branch conventions, a coding style guide, and loosely mentioned documentation. I think this document would solidly benefit by being split into at least three separate documents with a very clear purpose. For example, I'd break this document up into these:

1. Stacks network git branch maintenance protocol (feel free to rename but you get the idea)
2. Coding style guide
3. Documentation guide

And again, to the point above, each of these documents would further benefit from a call to action - explicit in the document or not - so that we can better place them going forward.

3. A quick glossary at the top of each document would be nice

4. The I/O Section could use more concrete details

The I/O section would benefit from an external justification or a case where using an I/O in the way this document warn against caused company problems or introduced a vulnerability. This way we have a tangible reason to trust this argument.

Additionally, I'm not clear on the exact kind of code that this document argues against. To elaborate on the issue with I/O and how to fix it the document could include a code snippet with impropper use of I/O and then a second code snippet below that has altered the code to have the same functionality without using I/O in a problematic way.

Overall, I love these principles and would love to see them in action!

[netrome]

I agree with the principles outlined here. This document is currently detached from the rest of the documentation. I suggest adding it under a Contributing section in SUMMARY.md. A way forward would be to add more contribution guidelines, including PR review guidelines which should point to these best practices.

[wileyj]

I agree with the principles outlined here. This document is currently detached from the rest of the documentation. I suggest adding it under a Contributing section in SUMMARY.md. A way forward would be to add more contribution guidelines, including PR review guidelines which should point to these best practices.

Hmm, i would argue this document should be standalone - but i like the idea of linking to it from SUMMARY.md

[netrome]

I agree with the principles outlined here. This document is currently detached from the rest of the documentation. I suggest adding it under a Contributing section in SUMMARY.md. A way forward would be to add more contribution guidelines, including PR review guidelines which should point to these best practices.

Hmm, i would argue this document should be standalone - but i like the idea of linking to it from SUMMARY.md

If it's standalone I think we should find another repo for it since this repo is dedicated for the sbtc-docs at https://stacks-network.github.io/sbtc-docs/

[wileyj]

I agree with the principles outlined here. This document is currently detached from the rest of the documentation. I suggest adding it under a Contributing section in SUMMARY.md. A way forward would be to add more contribution guidelines, including PR review guidelines which should point to these best practices.

Hmm, i would argue this document should be standalone - but i like the idea of linking to it from SUMMARY.md

If it's standalone I think we should find another repo for it since this repo is dedicated for the sbtc-docs at https://stacks-network.github.io/sbtc-docs/

maybe i'm missing something - but the current format of https://github.com/stacks-network/sbtc-docs/blob/master/src/SUMMARY.md is to just link to other documents. what i read in your comment is that the document being PR'ed here should be added to that SUMMARY.md file (not as a link, but as raw text). I agree it should be added to that file, but my comment was more that it should just be a link to the document here, vs copying the markdown directly to that file (please correct me if that assumption is incorrect).

Moving this document is also an option - we have the https://github.com/stacks-network/stacks repo where it could live, but i would still say it should be linked from SUMMARY.md.

[netrome]

maybe i'm missing something - but the current format of https://github.com/stacks-network/sbtc-docs/blob/master/src/SUMMARY.md is to just link to other documents. what i read in your comment is that the document being PR'ed here should be added to that SUMMARY.md file (not as a link, but as raw text). I agree it should be added to that file, but my comment was more that it should just be a link to the document here, vs copying the markdown directly to that file (please correct me if that assumption is incorrect).

Moving this document is also an option - we have the https://github.com/stacks-network/stacks repo where it could live, but i would still say it should be linked from SUMMARY.md.

So what I'm saying is that the document should be linked in SUMMARY.md. If it's not linked in that file, it's not going to be rendered in the documentation when served with mdbook.

[stjepangolemac]

I agree with everything that's mentioned in the document. I also think that sbtc-docs might not be the right place for this. I suggest two alternatives options:

• if you want to use this for Stacks in general - stacks-blockchain repo
• if you want to use this for sbtc development - sbtc repo

[jferrant]

THis is very clean and concise. LGTM. I feel like it should be contributing guidelines in all our repos basically, but wouldn't want to copy paste it to each if I didn't have to. So not sure where this should best live.

[wileyj]

shipit

[sergey-shandar]

1. How do we expect to use this document and what is the tangible outcome we'd like to see?

I've added an additional paragraph that describes the intent of the document.

2. It would be nice to split these standards into separate documents.

In one of the next PR: #83

4. The I/O Section could use more concrete details

I'm trying to keep it simple and short in the document but I'm working on an article I/O is a virus and would like to publish it in multiple places to get feedback from the broader community.

@AshtonStephens feel free to re-review and don't hesitate to open issues so we can make changes incrementally

[sergey-shandar]

@stjepangolemac , @jferrant

#84

[AshtonStephens]

Closing because this wasn't changed and doesn't have a clear purpose.