Skip to content

Commit

Permalink
Merge #920: [Docs] Overhaul documentation files
Browse files Browse the repository at this point in the history 
d24742c Remove redundant docs (Fuzzbawls)
007cc8c Update src/test/README.md (Fuzzbawls)
d2af959 Introduce translation_strings_policy.md (Fuzzbawls)
019b8f4 Update contrib/devtools/README.md (Fuzzbawls)
2bc9d1b Update developer-notes.md (Fuzzbawls)
d1a65fd Update dnsseed-policy.md (Fuzzbawls)
0b5c0c2 Update release-process.md (Fuzzbawls)
9335a10 Update test/functional/README.md (Fuzzbawls)
20f2513 Update doc/README.md (Fuzzbawls)
5b4f73d Clean up whitespace in Doxyfile.in (Fuzzbawls)
3cea76b Update and reformat init.md (Fuzzbawls)
2f5efbd Reformat files.md (Fuzzbawls)
df0d2ba Introduce dependencies.md (Fuzzbawls)
785b61c Update zmq.md (Fuzzbawls)
231f28e Update translation_process.md (Fuzzbawls)
c5f6822 Update tor.md (Fuzzbawls)
b7d0ec7 Fix markdown in REST-interface.md (Fuzzbawls)
4fa4cc4 Update Contributing guidelines (Fuzzbawls)
a544132 Update build notes (Fuzzbawls)

Tree-SHA512: ab422791c3e43d6e455d648c9b6b30943c176e03d2a0702135184108c8887bb39e4bb9c51bab9f9a04a130a297f8db879563e3b45f010c6b1f34482122b78681
  • Loading branch information
@Fuzzbawls
Fuzzbawls committed Jun 18, 2019
2 parents 95b584e + d24742c commit a56cc29
Show file tree
Hide file tree
Showing 28 changed files with 1,636 additions and 1,223 deletions.
97 changes: 73 additions & 24 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ facilitates social contribution, easy testing and peer review.

To contribute a patch, the workflow is as follows:

- Fork repository
- Create topic branch
- Commit patches
1. Fork repository
2. Create topic branch
3. Commit patches

The project coding conventions in the [developer notes](doc/developer-notes.md)
must be adhered to.
Expand All @@ -40,12 +40,14 @@ Commit messages should be verbose by default consisting of a short subject line
paragraph(s), unless the title alone is self-explanatory (like "Corrected typo
in init.cpp") in which case a single title line is sufficient. Commit messages should be
helpful to people reading your code in the future, so explain the reasoning for
your decisions. Further explanation [here](http://chris.beams.io/posts/git-commit/).
your decisions. Further explanation [here](https://chris.beams.io/posts/git-commit/).

If a particular commit references another issue, please add the reference, for
example `refs #1234`, or `fixes #4321`. Using the `fixes` or `closes` keywords
If a particular commit references another issue, please add the reference. For
example: `refs #1234` or `fixes #4321`. Using the `fixes` or `closes` keywords
will cause the corresponding issue to be closed when the pull request is merged.

Commit messages should never contain any `@` mentions.

Please refer to the [Git manual](https://git-scm.com/doc) for more information
about Git.

Expand Down Expand Up @@ -81,7 +83,11 @@ Examples:
Qt: Add feed bump button
Trivial: Fix typo in init.cpp

If a pull request is specifically not to be considered for merging (yet) please
Note that translations should not be submitted as pull requests, please see
[Translation Process](https://github.com/pivx-project/pivx/blob/master/doc/translation_process.md)
for more information on helping with translations.

If a pull request is not to be considered for merging (yet), please
prefix the title with [WIP] or use [Tasks Lists](https://help.github.com/articles/basic-writing-and-formatting-syntax/#task-lists)
in the body of the pull request to indicate tasks are pending.

Expand All @@ -94,6 +100,8 @@ At this stage one should expect comments and review from other contributors. You
can add more commits to your pull request by committing them locally and pushing
to your fork until you have satisfied all feedback.

Note: Code review is a burdensome but important part of the development process, and as such, certain types of pull requests are rejected. In general, if the **improvements** do not warrant the **review effort** required, the PR has a high chance of being rejected. It is up to the PR author to convince the reviewers that the changes warrant the review effort, and if reviewers are "Concept NAK'ing" the PR, the author may need to present arguments and/or do research backing their suggested changes.

Squashing Commits
---------------------------
If your pull request is accepted for merging, you may be asked by a maintainer
Expand All @@ -102,12 +110,16 @@ before it will be merged. The basic squashing workflow is shown below.

git checkout your_branch_name
git rebase -i HEAD~n
# n is normally the number of commits in the pull
# set commits from 'pick' to 'squash', save and quit
# on the next screen, edit/refine commit messages
# save and quit
# n is normally the number of commits in the pull request.
# Set commits (except the one in the first line) from 'pick' to 'squash', save and quit.
# On the next screen, edit/refine commit messages.
# Save and quit.
git push -f # (force push to GitHub)

Please update the resulting commit message if needed, it should read as a
coherent message. In most cases this means that you should not just list the
interim commits.

If you have problems with squashing (or other workflows with `git`), you can
alternatively enable "Allow edits from maintainers" in the right GitHub
sidebar and ask for help in the pull request.
Expand All @@ -120,11 +132,37 @@ the respective change set.
The length of time required for peer review is unpredictable and will vary from
pull request to pull request.

Rebasing Pull Requests
-------------------------
It may become necessary for a pull request to be rebased after other pull requests have been
merged. This is typically due to mutually exclusive changes (conflicts) between your pull
request and the current `master` branch.

When a rebase is needed, a comment will be added to the pull request indicating this need.
Rather than simply merge the `master` branch into your pull request (which results in an
ugly and confusing merge commit), it is better to use git's rebase feature. The basic
workflow is as follows:

# replace 'origin' with the remote name for the main project repo in the example
git checkout your_branch_name
git fetch origin
git pull --rebase origin master

This will "rewind" your branch commits, pull any new commits from `master`, then attempt to
re-apply your commits on top of the new HEAD. If any conflicts are found, the process will
pause and allow you to resolve any conflicts. Once conflicts have been resolved:

git rebase --continue

Repeat as necessary until there are no more conflicts and your git tree is in a clean state.
The final step is to push your rebased branch back up to github:

git push -f # force pushes the branch to github

Pull Request Philosophy
-----------------------

Patch sets should always be focused. For example, a pull request could add a
Patchsets should always be focused. For example, a pull request could add a
feature, fix a bug, or refactor code; but not a mixture. Please also avoid super
pull requests which attempt to do too much, are overly large, or overly complex
as this makes review difficult.
Expand All @@ -148,10 +186,18 @@ There are three categories of refactoring, code only moves, code style fixes,
code refactoring. In general refactoring pull requests should not mix these
three kinds of activity in order to make refactoring pull requests easy to
review and uncontroversial. In all cases, refactoring PRs must not change the
behavior of code within the pull request (bugs must be preserved as is).
behaviour of code within the pull request (bugs must be preserved as is).

Project maintainers aim for a quick turnaround on refactoring pull requests, so
where possible keep them short, un-complex and easy to verify.
where possible keep them short, uncomplex and easy to verify.

Pull requests that refactor the code should not be made by new contributors. It
requires a certain level of experience to know where the code belongs and to
understand the full ramification (including rebase effort of open pull requests).

Trivial pull requests or pull requests that refactor the code with no clear
benefits may be immediately closed by the maintainers to reduce unnecessary
workload on reviewing.


"Decision Making" Process
Expand All @@ -169,9 +215,9 @@ judge the general consensus of contributors.

In general, all pull requests must:

- have a clear use case, fix a demonstrable bug or serve the greater good of
- Have a clear use case, fix a demonstrable bug or serve the greater good of
the project (for example refactoring for modularisation);
- be well peer reviewed;
- Be well peer reviewed;
- follow code style guidelines;

Patches that change PIVX consensus rules are considerably more involved than
Expand All @@ -185,13 +231,16 @@ patches because of increased peer review and consensus building requirements.

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
test out the patchset 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, forums, email, and Slack discussions). The following
spread out over GitHub, forums, email, and Discord discussions). The following
language is used within pull-request comments:

- ACK means "I have tested the code and I agree it should be merged";
- (t)ACK means "I have tested the code and I agree it should be merged", involving
change-specific manual testing in addition to running the unit and functional
tests, and in case it is not obvious how the manual testing was done, it should
be described;
- NACK means "I disagree this should be merged", and must be accompanied by
sound technical justification (or in certain cases of copyright/patent/licensing
issues, legal justification). NACKs without accompanying reasoning may be
Expand All @@ -209,13 +258,13 @@ that have demonstrated a deeper commitment and understanding towards the project
(over time) or have clear domain expertise may naturally have more weight, as
one would expect in all walks of life.

Where a patch set affects consensus critical code, the bar will be set much
Where a patchset affects consensus critical code, the bar will be set much
higher in terms of discussion and peer review requirements, keeping in mind that
mistakes could be very costly to the wider community. This includes refactoring
of consensus critical code.

Where a patch set proposes to change the PIVX consensus, it must have been
discussed extensively on the forums and Slack, be accompanied by a widely
Where a patchset proposes to change the PIVX consensus, it must have been
discussed extensively on the forums and Discord, be accompanied by a widely
discussed Proposal and have a generally widely perceived technical consensus of being
a worthwhile change based on the judgement of the maintainers.

Expand All @@ -237,15 +286,15 @@ about:
that personally, though! Instead, take another critical look at what you are suggesting
and see if it: changes too much, is too broad, doesn't adhere to the
[developer notes](doc/developer-notes.md), is dangerous or insecure, is messily written, etc.
Identify and address any of the issues you find. Then ask e.g. on Slack if someone could give
Identify and address any of the issues you find. Then ask e.g. on Discord if someone could give
their opinion on the concept itself.
- It may be because your code is too complex for all but a few people. And those people
may not have realized your pull request even exists. A great way to find people who
are qualified and care about the code you are touching is the
[Git Blame feature](https://help.github.com/articles/tracing-changes-in-a-file/). Simply
find the person touching the code you are touching before you and see if you can find
them and give them a nudge. Don't be incessant about the nudging though.
- Finally, if all else fails, ask on Slack or elsewhere for someone to give your pull request
- Finally, if all else fails, ask on Discord or elsewhere for someone to give your pull request
a look. If you think you've been waiting an unreasonably long amount of time (month+) for
no particular reason (few lines changed, etc), this is totally fine. Try to return the favor
when someone else is asking for feedback on their code, and universe balances out.
Expand Down
157 changes: 128 additions & 29 deletions contrib/devtools/README.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Contents
===========
========
This directory contains tools for developers working on this repository.

check-doc.py
Expand All @@ -8,6 +8,93 @@ check-doc.py
Check if all command line args are documented. The return value indicates the
number of undocumented args.

clang-format-diff.py
===================

A script to format unified git diffs according to [.clang-format](../../src/.clang-format).

Requires `clang-format`, installed e.g. via `brew install clang-format` on macOS.

For instance, to format the last commit with 0 lines of context,
the script should be called from the git root folder as follows.

```
git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v
```

copyright\_header.py
====================

Provides utilities for managing copyright headers of `The PIVX
developers` in repository source files. It has three subcommands:

```
$ ./copyright_header.py report <base_directory> [verbose]
$ ./copyright_header.py update <base_directory>
$ ./copyright_header.py insert <file>
```
Running these subcommands without arguments displays a usage string.

copyright\_header.py report \<base\_directory\> [verbose]
---------------------------------------------------------

Produces a report of all copyright header notices found inside the source files
of a repository. Useful to quickly visualize the state of the headers.
Specifying `verbose` will list the full filenames of files of each category.

copyright\_header.py update \<base\_directory\> [verbose]
---------------------------------------------------------
Updates all the copyright headers of `The PIVX developers` which were
changed in a year more recent than is listed. For example:
```
// Copyright (c) <firstYear>-<lastYear> The PIVX developers
```
will be updated to:
```
// Copyright (c) <firstYear>-<lastModifiedYear> The PIVX developers
```
where `<lastModifiedYear>` is obtained from the `git log` history.

This subcommand also handles copyright headers that have only a single year. In
those cases:
```
// Copyright (c) <year> The PIVX developers
```
will be updated to:
```
// Copyright (c) <year>-<lastModifiedYear> The PIVX developers
```
where the update is appropriate.

copyright\_header.py insert \<file\>
------------------------------------
Inserts a copyright header for `The PIVX developers` at the top of the
file in either Python or C++ style as determined by the file extension. If the
file is a Python file and it has `#!` starting the first line, the header is
inserted in the line below it.

The copyright dates will be set to be `<year_introduced>-<current_year>` where
`<year_introduced>` is according to the `git log` history. If
`<year_introduced>` is equal to `<current_year>`, it will be set as a single
year rather than two hyphenated years.

If the file already has a copyright for `The PIVX developers`, the
script will exit.

gen-manpages.sh
===============

A small script to automatically create manpages in ../../doc/man by running the release binaries with the -help option.
This requires help2man which can be found at: https://www.gnu.org/software/help2man/

With in-tree builds this tool can be run from any directory within the
repostitory. To use this tool with out-of-tree builds set `BUILDDIR`. For
example:

```bash
BUILDDIR=$PWD/build contrib/devtools/gen-manpages.sh
```

github-merge.py
===============

Expand Down Expand Up @@ -40,43 +127,44 @@ Configuring the github-merge tool for the PIVX repository is done in the followi

git config githubmerge.repository PIVX-Project/PIVX
git config githubmerge.testcmd "make -j4 check" (adapt to whatever you want to use for testing)
git config --global user.signingkey mykeyid (if you want to GPG sign)

optimize-pngs.py
================

A script to optimize png files in the PIVX
repository (requires pngcrush).
git config --global user.signingkey mykeyid

fix-copyright-headers.py
===========================
Authentication (optional)
--------------------------

Every year newly updated files need to have its copyright headers updated to reflect the current year.
If you run this script from src/ it will automatically update the year on the copyright header for all
.cpp and .h files if these have a git commit from the current year.
The API request limit for unauthenticated requests is quite low, but the
limit for authenticated requests is much higher. If you start running
into rate limiting errors it can be useful to set an authentication token
so that the script can authenticate requests.

For example a file changed in 2014 (with 2014 being the current year):
```// Copyright (c) 2009-2013 The Bitcoin developers```
- First, go to [Personal access tokens](https://github.com/settings/tokens).
- Click 'Generate new token'.
- Fill in an arbitrary token description. No further privileges are needed.
- Click the `Generate token` button at the bottom of the form.
- Copy the generated token (should be a hexadecimal string)

would be changed to:
```// Copyright (c) 2009-2014 The Bitcoin developers```
Then do:

logprint-scanner.py
===================
LogPrint and LogPrintf are known to throw exceptions when the number of arguments supplied to the
LogPrint(f) function is not the same as the number of format specifiers.
git config --global user.ghtoken "pasted token"

Ideally, the presentation of this mismatch would be at compile-time, but instead it is at run-time.
Create and verify timestamps of merge commits
---------------------------------------------
To create or verify timestamps on the merge commits, install the OpenTimestamps
client via `pip3 install opentimestamps-client`. Then, dowload the gpg wrapper
`ots-git-gpg-wrapper.sh` and set it as git's `gpg.program`. See
[the ots git integration documentation](https://github.com/opentimestamps/opentimestamps-client/blob/master/doc/git-integration.md#usage)
for further details.

This script scans the src/ directory recursively and looks in each .cpp/.h file and identifies all
errorneous LogPrint(f) calls where the number of arguments do not match.
optimize-pngs.py
================

The filename and line number of the errorneous occurence is given.
A script to optimize png files in the PIVX
repository (requires pngcrush).

The script returns with the number of erroneous occurences as an error code to help facilitate
integration with a continuous integration system.
security-check.py and test-security-check.py
============================================

The script can be ran from any working directory inside the git repository.
Perform basic ELF security checks on a series of executables.

symbol-check.py
===============
Expand All @@ -87,7 +175,7 @@ still compatible with the minimum supported Linux distribution versions.

Example usage after a gitian build:

find ../gitian-builder/build -type f -executable | xargs python contrib/devtools/symbol-check.py
find ../gitian-builder/build -type f -executable | xargs python3 contrib/devtools/symbol-check.py

If only supported symbols are used the return value will be 0 and the output will be empty.

Expand All @@ -109,3 +197,14 @@ It will do the following automatically:
- add missing translations to the build system (TODO)

See doc/translation-process.md for more information.

circular-dependencies.py
========================

Run this script from the root of the source tree (`src/`) to find circular dependencies in the source code.
This looks only at which files include other files, treating the `.cpp` and `.h` file as one unit.

Example usage:

cd .../src
../contrib/devtools/circular-dependencies.py {*,*/*,*/*/*}.{h,cpp}
Loading

0 comments on commit a56cc29

Please sign in to comment.