[bisq-network/bisq] Update contribution guidelines (#3762)
Julian Knutsen
notifications at github.com
Tue Dec 10 18:46:52 UTC 2019
julianknutsen commented on this pull request.
Small comments inline. I tried not to go too far in the weeds with the feedback I've already shared with my onboarding experience. Happy to take another look for more structure/grammar issues once a few more people weigh in.
>
-Discussion about code changes happens in GitHub issues and pull requests.
+**Will you get paid?** Well, yes and no. With Bisq, there is no classic funding or investors, so there is no payout of fiat money.
+- Yes, because there is the DAO. The DAO allows for compensation requests to be filed and voted on. If such a voting goes in your favor, you receive BSQ, a colored bitcoin token. Sell your BSQ and you get your fiat eventually.
I would suggest an in-page link for the text "compensation requests" that go to the relevant section. Getting up to speed on all the logistics of a brand-new project can be overwhelming and it is likely that this document will be skimmed a few dozen times the first week someone is on the project. Quick navigation to the most important points that will help devs decide if this is a good project should be a priority.
>
-Discussion about code changes happens in GitHub issues and pull requests.
+**Will you get paid?** Well, yes and no. With Bisq, there is no classic funding or investors, so there is no payout of fiat money.
+- Yes, because there is the DAO. The DAO allows for compensation requests to be filed and voted on. If such a voting goes in your favor, you receive BSQ, a colored bitcoin token. Sell your BSQ and you get your fiat eventually.
+- No, because the market is still very small. We can sustain a few full-timers (paying rent, tax, small stuff) but do not expect to get rich fast and easy just now.
Full/part-time varies drastically across the world and may give the wrong impression.
@ripcurlx It also seems strange that full-timers wouldn't be preferred. Full-time developers are certainly more efficient, can work on more impactful projects, and can get up to speed much faster with fewer resources. If that isn't the goal, it may make more sense for this doc to call out small & part-time as the target contributor.
I think the best bet is to provide easy access to the metrics so devs can decide for themselves if it makes sense for their situation. I don't even think this exists in one-place right now, but the most useful item for me when analyzing this was to look at the total amount of BSQ purchased over the last 30 days and the average price. This gave me some understanding of how much demand there was for my future compensation request BSQ and how much would need to stay invested.
That, along with an easy way for people to view the end-of-cycle reports with the major inflation/deflation stats give a good glimpse of the state of BSQ.
Finally, this may exist in some other docs, but explaining that not only is there a free market for BSQ that traders use for reduced fees, but BSQ is also actively purchased from BTC fees and Arbitration every month. As the project grows, the liquidity will grow with it.
Not every dev is going to have a solid grasp on the economics so helping them get the high-level information is important so they don't get too invested with something that doesn't work for them.
I'm not a huge fan of the wording "So do not expect to get rich fast and easy just now.". Is the goal to discourage speculative investors? Devs asking for more than their worth?
>
-Discussion about larger changes to the way Bisq works happens in issues the [bisq-network/proposals](https://github.com/bisq-network/proposals/issues) repository. See https://docs.bisq.network/proposals.html for details.
+The more correct way to see payment is "compensation". By earning BSQ, you invest in the Bisq project (just like other startups do it). But make no mistake, the DAO has only been operating for a few months now. Eventually, Bisq will create serious revenue.
`But make no mistake, the DAO has only been operating for a few months now. Eventually, Bisq will create serious revenue.`
It isn't clear what `revenue` is referring to in the line. Does that translate to higher BSQ value or the capacity to "employ" more people at once?
I think @ripcurlx has a better phrasing here by suggesting that the gap between burnt and issued __could__ decrease as the project is more successful. But, the DAO may decide to use increased BSQ burn rate to employ more people instead of deflating supply so the future is really unknown.
If I were looking at this with fresh eyes again, I would want a pointer to the actual data that I could analyze and extrapolate. Is there a chart of earned vs. burned that is accessible since the lifetime of BSQ? What about the growth rate of total transaction volume or trading fees? Anything that helps forecast the future of Bisq's revenue vs expenses.
>
+**Tools?** If you want to develop code, we expect you to know your ways around Git and Java. Then comes testing, JavaFX, Gradle, Github, ... As for non-functional skills, we love to see people knowing their ways around P2P networks, cryptography, security, privacy, ...
`Then comes testing, JavaFX, Gradle, Github, ...`
I expected a list of testing frameworks or patterns after the first three words. I would suggest including either the specific testing framework that Bisq uses or split out the fact that strong candidates for Bisq know how to test code.
The non-functional skills part doesn't seem to fit with Tools. It might make sense to fold that into another section.
>
-## Contributor Workflow
+**Carreer?** Start small, go bigger. If we like your work, we might just ask you to take on a more official role. Maybe someday you are the guy improving this document (again).
Who is "we"? I thought decisions were made from the DAO? What are "official roles"?
These are the first questions that came into my head when reading this. What is the intention of this section?
If it is to explain that Bisq is looking for developers to start small and part-time that might be good to add explicitly.
If it is to explain that Bisq only wants people familiar with open-source development structure it might be worth including explicitly.
If it is to explain that Bisq isn't a typical hierarchical organization with career paths it may make sense to include a section on meritocracy or the concept of distributed asynchronous dev orgs.
>
-Anyone may participate in peer review which is expressed by comments in the pull request. Typically reviewers will review the code for obvious errors, as well as test out the patch set and opine on the technical merits of the patch. Project maintainers take into account the peer review when determining if there is consensus to merge a pull request (remember that discussions may have been spread out over GitHub, mailing list and IRC discussions). The following language is used within pull-request comments:
+Now you are ready to roll. Pick something off the [Good First Issue List](https://github.com/bisq-network/bisq/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22), pick any other issue, do some testing, spot a bug, report it and fix it, maybe there is something in the UI that needs cosmetic work.
When you think of the first commit by the perfect Bisq contributor candidate what is it and where is it?
This lists 5 different categories without any sense of preference. Each one is going to have a very different set of resources required by PRs, testing, etc.
It might make sense to shorten it up so that onboarding is easier for reviewers and contributors.
>
- - `ACK` means "I have tested the code and I agree it should be merged";
- - `NACK` means "I disagree this should be merged", and must be accompanied by sound technical justification. NACKs without accompanying reasoning may be disregarded;
- - `utACK` means "I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged";
- - `Concept ACK` means "I agree in the general principle of this pull request";
- - `Nit` refers to trivial, often non-blocking issues.
-
-Please note that Pull Requests marked `NACK` and/or GitHub's `Change requested` are closed after 30 days if not addressed.
-
-
-## Compensation
+Now that you have picked yourself something to work on, be vocal about it! Claim good first issues, or any issues, by simply leaving a comment saying that this is being worked on, join [Keybase](https://keybase.io/team/bisq). Once there, introduce yourself in *#introductions*, get help in *#dev-onboarding* if your setup fights you, use *#dev* to see if the task you picked is a good way forward (you do not have to do that for good first issues) and join other channels if you want to.
and register your github with keybase
>
-See https://help.github.com/articles/setting-your-username-in-git/ for instructions.
+## Honor Git best practice
+ - Create a topic branch off master
+ - Commit little, commit often
+ - Maintain a clean commit history (no merges commits!, use rebase and force-push if necessary)
`no merge commits`
>
- 1. Separate subject from body with a blank line
- 2. Limit the subject line to 50 characters
- 3. Capitalize the subject line
- 4. Do not end the subject line with a period
- 5. Use the imperative mood in the subject line
- 6. Wrap the body at 72 characters
- 7. Use the body to explain what and why vs. how
+## Test your code!
+ - Create automated tests for your work. JUnit is available. If you are fixing a bug, create a test that demonstrates the bug by failing. Then fix the bug.
+ - If automated testing is not feasible (we know it can be hard - working on it ...), provide a textual testing procedure (which we can include in our manual testing checklist): setup, prerequisits, steps, expected results
+ - If that is not possible either, provide some info (numbers, screenshots, testing procedure) on how you tested your stuff
+ - If all is lost, state why you have not been able to do the above.
I like this section. The burden is on the contributor to convince the reviewers that the code is solid. Not the other way around.
>
-### Additional style guidelines
+Your pull request got merged? great, congrats and thanks for making Bisq more better.
```suggestion
Your pull request got merged? Great! Congrats on your first contribution and thanks for making Bisq better.
```
>
- - `ACK` means "I have tested the code and I agree it should be merged";
- - `NACK` means "I disagree this should be merged", and must be accompanied by sound technical justification. NACKs without accompanying reasoning may be disregarded;
- - `utACK` means "I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged";
- - `Concept ACK` means "I agree in the general principle of this pull request";
- - `Nit` refers to trivial, often non-blocking issues.
-
-Please note that Pull Requests marked `NACK` and/or GitHub's `Change requested` are closed after 30 days if not addressed.
-
-
-## Compensation
+Now that you have picked yourself something to work on, be vocal about it! Claim good first issues, or any issues, by simply leaving a comment saying that this is being worked on, join [Keybase](https://keybase.io/team/bisq). Once there, introduce yourself in *#introductions*, get help in *#dev-onboarding* if your setup fights you, use *#dev* to see if the task you picked is a good way forward (you do not have to do that for good first issues) and join other channels if you want to.
Link to the calendar of events for design, growth, dev-calls, and even daily standup?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3762#pullrequestreview-329982666
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191210/26e37c12/attachment-0001.html>
More information about the bisq-github
mailing list