Page MenuHomePhabricator
Differential D750 Diff 1928 doc/abc-developer-guide.md

Changeset View

Standalone View

View Options

doc/abc-developer-guide.md

  • This file was added.
Bitcoin ABC Development Guide
=============================
This guide is intended to help developers get started contributing to Bitcoin ABC.
Bitcoin ABC Development Philosophy
----------------------------------
Bitcoin ABC aims for fast iteration and continuous integration.
This means that there should be quick turnaround for patches to be proposed,
reviewed, and committed.
Here are some tips to help keep the development working as intended:
- Keep each change small and self-contained.
schancelUnsubmitted
Not Done
Inline Actions

This will be confusing for a lot of developers. I do not normally work this way, and it's forcing me to rethink how I go about manipulating the code. Although, I think it's for the better. We should discuss in depth what we mean, and how it improves quality. It's sort of a measure-twice cut-once type deal.

schancel: This will be confusing for a lot of developers. I do not normally work this way, and it's…
MengerianAuthorUnsubmitted
Not Done
Inline Actions

I'm trying got capture how Amaury wants people to work in the project.

I won't land the diff unless he is happy with this part

Mengerian: I'm trying got capture how Amaury wants people to work in the project. I won't land the diff…
jasonbcoxUnsubmitted
Not Done
Inline Actions

For what it's worth, we have this policy at LinkedIn. Smaller diffs (usually) influence the following:

  • Easier, more digestible chunks that can be reviewed by reviewers in a single sitting. This makes it more likely to get quicker reviews, especially if you require reviews from multiple people.
  • Less likely to run into large merge conflicts.
  • Having one or more test changes/additions per diff encourages code coverage* (*This doesn't apply to all diffs, such as refactors that display the same behavior and are covered by existing tests)
  • Writing code in small chunks can influence how you design your changes from a higher level. Personally, I've found it to be much easier to pivot and make design changes as you get reviews on small changes. For example, if you know you'll be working on a large change, committing skeleton code with /*TODO: <some short description*/ where appropriate can make your design intentions clear without wasting your time on the implementation details. This is especially effective when the design hasn't been agreed on or is not well understood.

Just my two cents.

jasonbcox: For what it's worth, we have this policy at LinkedIn. Smaller diffs (usually) influence the…
- Land the Diff quickly after it is reviewed.
freetraderUnsubmitted
Done
Inline Actions

after it is accepted?

freetrader: after it is accepted?
- Review Diffs from other developers as quickly as possible.
schancelUnsubmitted
Done
Inline Actions

I want to encourage people to reach out for a 1-on-1 review so things move quickly

schancel: I want to encourage people to reach out for a 1-on-1 review so things move quickly
- Large changes should be broken into logical chunks that are easy to review,
and keep the code in a functional state.
- There are no "development" branches, all Diffs apply to the Master
branch, and should always improve it (no regressions).
- Don't break the build, and immediately revert Diffs that do break it.
- Automate as much as possible, and spend time on things only humans can do.
Here are some handy links for development practices aligned with Bitcoin ABC:
- [Statement of Bitcoin ABC Values and Visions](https://www.yours.org/content/bitcoin-abc---our-values-and-vision-a282afaade7c)
- [Large Diffs Are Hurting Your Ability To Ship](https://medium.com/@kurtisnusbaum/large-diffs-are-hurting-your-ability-to-ship-e0b2b41e8acf)
- [Advantages of monolithic version control](https://danluu.com/monorepo/)
- [Stacked Diffs: Keeping Phabricator Diffs Small](https://medium.com/@kurtisnusbaum/stacked-diffs-keeping-phabricator-diffs-small-d9964f4dcfa6)
- [The Pragmatic Programmer: From Journeyman to Master](https://www.amazon.com/Pragmatic-Programmer-Journeyman-Master/dp/020161622X)
Getting set up with the Bitcoin ABC Repository
jasonbcoxUnsubmitted
Not Done
Inline Actions

Seems like this section duplicates a lot from docs/developer-notes, including some notes in this outstanding review: https://reviews.bitcoinabc.org/D720 Is this file intended to replace developer-notes?

jasonbcox: Seems like this section duplicates a lot from docs/developer-notes, including some notes in…
MengerianAuthorUnsubmitted
Not Done
Inline Actions

Good point, I didn't notice D720.

We should avoid duplication.

Mengerian: Good point, I didn't notice D720. We should avoid duplication.
jasonbcoxUnsubmitted
Not Done
Inline Actions

FYI I just pushed D720, so feel free to rebase and move that content if you think it fits better in CONTRIBUTING.md

jasonbcox: FYI I just pushed D720, so feel free to rebase and move that content if you think it fits…
MengerianAuthorUnsubmitted
Not Done
Inline Actions

OK, yeah, I think it's better to have this stuff in CONTRIBUTING.md, so I'll move things over as appropriate.

doc/developer-notes.md is full of old Core stuff, a lot of it should probably be gotten rid of, so it's not the best place for useful information right now.

Mengerian: OK, yeah, I think it's better to have this stuff in CONTRIBUTING.md, so I'll move things over…
----------------------------------------------
1. Create an account at https://reviews.bitcoinabc.org/
2. Install Git and Arcanist on your machine
`sudo apt-get install git arcanist`
freetraderUnsubmitted
Done
Inline Actions

This is very platform dependent - would be good if the doc pointed this out and gave links to the Phabricator user documentation, e.g.

https://secure.phabricator.com/book/phabricator/article/arcanist_quick_start/

and the "Installing Arcanist" section in

https://secure.phabricator.com/book/phabricator/article/arcanist/

freetrader: This is very platform dependent - would be good if the doc pointed this out and gave links to…
3. If you do not already have an SSH key set up, follow these steps:
Type: `ssh-keygen -t rsa -b 4096 -C "your_email@example.com"`
Enter a file in which to save the key (/home/*username*/.ssh/id_rsa): [Press enter]
4. Upload your SSH key to reviews.bitcoinabc.org
freetraderUnsubmitted
Done
Inline Actions

SSH key -> SSH public key

freetrader: SSH key -> SSH public key
- go to: `https://reviews.bitcoinabc.org/settings/user/*username*/page/ssh/`
- Under "SSH Key Actions", Select "Upload Public Key"
Paste contents from: `/home/*username*/.ssh/id_rsa.pub`
5. Clone the repository and install Arcanist certificate:
```
git clone ssh://vcs@reviews.bitcoinabc.org:2221/source/bitcoin-abc.git
cd bitcoin-abc
arc install-certificate
```
Then paste an API token from: `https://reviews.bitcoinabc.org/settings/user/*username*/page/apitokens/`
Working with Phabricator and Arcanist
freetraderUnsubmitted
Done
Inline Actions

There are some prerequisite tools for linting that are needed depending on what files are being modified.

Known to me are clang-format-3.8 and autopep8 .

freetrader: There are some prerequisite tools for linting that are needed depending on what files are being…
MengerianAuthorUnsubmitted
Done
Inline Actions

I think they installed automatically for me in Ubuntu.

I will add a comment though, to mention that people may have to install them if it doesn't happen automatically

Mengerian: I think they installed automatically for me in Ubuntu. I will add a comment though, to mention…
-------------------------------------
A typical workflow would be:
- Create a topic branch in Git for your changes
`git checkout -b 'my-topic-branch'`
schancelUnsubmitted
Done
Inline Actions

FWIW, The single quotes are extraneous.

schancel: FWIW, The single quotes are extraneous.
- Make your changes, and commit them
`git commit -a -m 'my-commit'`
- Create a differential with Arcanist
`arc diff`
You should add suggested reviewers and a test plan to the commit message.
Note that Arcanist is set up to look only at the most-recent commit message,
So all you changes for this Diff should be in one Git commit.
- Log into Phabricator to see review and feedback.
- Make changes as suggested by the reviewers. You can simply edit the files
with my-topic-branch checked out, and then type `arc diff`. Arcanist will
give you the option to add uncommited changes. Or, alternatively, you can
commit the changes, and squash the commits by typing `git rebase -i HEAD~2`.
If you Squash, make sure the most recent commit message has the information
needed for arcanist (such as the Diff number, reviewers, etc.).
- When reviewers approve your Diff, it should be listed as "ready to Land"
in Phabricator. When you want to commit your diff to the repository, check out
type my-topic-branch in git, then type `arc land`. You have now succesfully
committed a change to the Bitcoin ABC repository.