Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Commit

Permalink
Add files for open sourcing (#22)
Browse files Browse the repository at this point in the history
  • Loading branch information
rain-on committed Apr 11, 2019
1 parent d0390aa commit c5160bb
Show file tree
Hide file tree
Showing 6 changed files with 963 additions and 0 deletions.
74 changes: 74 additions & 0 deletions CODE-OF-CONDUCT.md
@@ -0,0 +1,74 @@
# Contributor Covenant Code of Conduct

## Our Pledge

In the interest of fostering an open and welcoming environment, we as
contributors and maintainers pledge to making participation in our project and
our community a harassment-free experience for everyone, regardless of age, body
size, disability, ethnicity, sex characteristics, gender identity and expression,
level of experience, education, socio-economic status, nationality, personal
appearance, race, religion, or sexual identity and orientation.

## Our Standards

Examples of behavior that contributes to creating a positive environment
include:

* Using welcoming and inclusive language
* Being respectful of differing viewpoints and experiences
* Gracefully accepting constructive criticism
* Focusing on what is best for the community
* Showing empathy towards other community members

Examples of unacceptable behavior by participants include:

* The use of sexualized language or imagery and unwelcome sexual attention or
advances
* Trolling, insulting/derogatory comments, and personal or political attacks
* Public or private harassment
* Publishing others' private information, such as a physical or electronic
address, without explicit permission
* Other conduct which could reasonably be considered inappropriate in a
professional setting

## Our Responsibilities

Project maintainers are responsible for clarifying the standards of acceptable
behavior and are expected to take appropriate and fair corrective action in
response to any instances of unacceptable behavior.

Project maintainers have the right and responsibility to remove, edit, or
reject comments, commits, code, wiki edits, issues, and other contributions
that are not aligned to this Code of Conduct, or to ban temporarily or
permanently any contributor for other behaviors that they deem inappropriate,
threatening, offensive, or harmful.

## Scope

This Code of Conduct applies both within project spaces and in public spaces
when an individual is representing the project or its community. Examples of
representing a project or community include using an official project e-mail
address, posting via an official social media account, or acting as an appointed
representative at an online or offline event. Representation of a project may be
further defined and clarified by project maintainers.

## Enforcement

Instances of abusive, harassing, or otherwise unacceptable behavior may be
reported by contacting the project team at [private@pegasys.tech]. All
complaints will be reviewed and investigated and will result in a response that
is deemed necessary and appropriate to the circumstances. The project team is
obligated to maintain confidentiality with regard to the reporter of an incident.
Further details of specific enforcement policies may be posted separately.

Project maintainers who do not follow or enforce the Code of Conduct in good
faith may face temporary or permanent repercussions as determined by other
members of the project's leadership.

## Attribution

This Code of Conduct is adapted from the [Contributor Covenant], version 1.4,
available at https://www.contributor-covenant.org/version/1/4/code-of-conduct.html

[Contributor Covenant]: https://www.contributor-covenant.org
[private@pegasys.tech]: mailto:private@pegasys.tech
219 changes: 219 additions & 0 deletions CODING-CONVENTIONS.md
@@ -0,0 +1,219 @@
# Contents
* [1 Introduction](#1-introduction)
* [2 General Design Philosophy](#2-general-design-philosophy)
* [3 Specific Design Techniques](#3-specific-design-techniques)
* [4 Java](#4-java)
* [5 Logging](#5-logging)

# 1 Introduction

This document contains guidelines (some stricter than others) so we can be consistent and spend more time solving the bigger and more interesting issues.

The guidelines are intended to facilitate working together not to facilitate reviews that criticize without adding value.

Some guidelines are personal opinion. The idea being we make a decision once, document it, and apply it for consistency. Again, we can then spend more time on the interesting issues and less time discussing coding conventions :-)

# 2 General Design Philosophy

The key principles are:
* Keep It Simple
* Idiomatic Java
* YAGNI (You Ain't Gonna Need It)

## 2.1 Keep It Simple

Simple does not mean the fewest lines of code. Simple code is:
* Easy to understand
* Self-documenting and not dependent on comments to explain what it does
* Understandable even to inexperienced Java developers
* Dependent on selecting the right data structures for the task
* Usually the most performant. Without data showing another approach is faster, stick with the simple design
* Not simplistic:

- Ethereum is complex and EthSigner must handle this complexity and operate correctly and securely
- EthSigner code should align with well-established Ethereum abstractions and terminology used in Ethereum specifications
- Aim to make the code as simple as possible but no simpler

## 2.2 Idiomatic Java

EthSigner embraces typical Java idioms including using an Object Oriented approach to design. This includes:
* Providing alternate behaviours via polymorphism instead of having conditional logic scattered through the codebase. For example, `ProtocolSpec` provides a standard interface to blockchain operations and multiple implementations define the different behaviours for each Ethereum milestone.
* Encapsulating behaviour and data together in classes. For example, `BytesValue` encapsulates byte data and methods operating on the byte data. `BytesValue.isZero()` is an instance method instead of accepting a `BytesValue` parameter.

`ProtocolSpec` is an exception and does not hold the blockchain data on which it operates. This is because that blockchain data is widely shared and not specifically owned by `ProtocolSpec`.
* Embracing modern Java features like Optional, Streams and lambdas when they make code simpler and clearer.

- Do use Streams and map with lambdas to convert values in a list to a different form.
- Don't pass lambdas into executors because it makes it harder to identify the threading interactions. The lambda makes the code shorter but not clearer. Instead use a separate class or extract a method.
* For good examples, refer to the APIs the JDK itself exposes.

>**Note** If you're not sure what idiomatic Java looks like, start by following the typical patterns and naming used in EthSigner, or otherwise, other Pegasys codebases.
## 2.3 You Ain't Gonna Need It (YAGNI)

The EthSigner design prioritizes meeting current requirements in the simplest, clearest way over attempting to anticipate future functionality. As a result, EthSigner’s design:
* Is not set in stone as a big upfront design. The design is adjusted through constant refactoring as new requirements are added and understood.
* Uses abstraction only where it aids understanding of the current code. Abstraction is not used where it only supports future needs.
* Avoids over-engineering.

# 3 Specific Design Techniques

# 3.1 Creating Understandable, Self-documenting Code

To create understandable, self-documenting code:
- Use clear naming for variables, methods, and classes
- Use US spelling instead of UK. For example, synchronize instead of synchronise
- Keep methods and classes short and focused on a single responsibility. Preferable maximum lengths:
* Lambdas: 1 - 3 lines
* Methods: less than 50 lines
* Anonymous classes: less than 50 lines
* Inner classes: not much more than 50 lines
* Classes: a few hundred lines
- Be thoughtfully organised in terms of method order, package structure, and module usage
- Follow well-established patterns and conventions
- Be consistent
- Make it easy to follow the control flow by _clicking through_ in an IDE
- Make it easier to use the right way than the wrong way
- Avoid abbreviations. We are a global team and when English is a second language abbreviations reduce readability. The following abbreviations are exceptions:
* tx -> Transaction
* RPC -> Remote Procedure Call

# 3.2 Creating Code for Constant Refactoring and Evolving Design

So the code can cope with constant refactoring and evolving design, write code that:
* Is well tested.

Avoid test cases requiring detailed interactions with mocks because these are often brittle.

* Avoids duplication.

* Follows the Single Responsibility Principle where each class is responsible for one thing.

>**Note** It is important to scope the responsibility wisely. Responsibilities that are:
> * Too small lead to an explosion of classes making things hard to follow
> * Too large lead to the class becoming big and unwieldy
* Favors composition over inheritance. You can use inheritance, but prefer composition

* Uses dependency injection
- Constructors should be simple, with dependencies passed in rather than built in the constructor
- EthSigner does not use a dependency injection framework

* Validates method parameters for public methods using the Guava `Preconditions` class. Avoid validating parameters in private methods

* Generally avoids interfaces with a single implementation unless they are explicitly being used to narrow the exposed API

* Uses the most general applicable type. For example, `List` or `Collection` instead of `ArrayList`

## 3.3 Additional Design Elements


* Use Optional rather than returning null when not having a value is a normal case

* Consider exception and error handling as part of the overall design. EthSigner avoids checked exceptions

* Give threads meaningful names. For example:
`Executors.newFixedThreadPool(1, new ThreadFactoryBuilder().setNameFormat(“Ibft”).build())`


# 4 Java

## 4.1 Style Guide

EthSigner follows the [Google code style](https://google.github.io/styleguide/javaguide.html) and uses spotless to ensure consistency of formatting.

To automatically reformat the code before creating a pull request, run:

```json
./gradlew spotlessApply
```

### 4.1.1 Install Google Style Settings

The Google style settings can be installed in [Intellij](https://github.com/google/google-java-format#intellij) and [Eclipse](https://github.com/google/google-java-format#eclipse).

## 4.2 Additional Java Style Guidelines

## 4.2.1 Fields

Class-level fields are generally not separated by blank lines but can use a blank line to separate them into logical groupings.

## 4.2.2 Final Keyword

Method parameters must be final. Class level and local fields should be final whenever possible.

## 4.2.3 Common Methods

* Getters follow idiomatic format with `get` prefix. For example, `getBlock()` gets a block property.
* Setters follow idiomatic format with `set` prefix. For example, `setBlock(Block block)` sets a block property.
* For `toString methods`, use the Guava 18+ `MoreObjects.toStringHelper`
* Equals and `hashCode()` methods use the `Object.equals` and `Object.hash` methods (this is the _Java 7+_ template in IntelliJ. Don’t accept subclasses and don’t use getters)

## 4.2.4 Testing

* Don't use a fixed prefix (for example, `test`). Do explain the expected behaviour not just the situation

Good: `returnTrueWhenValueIsXyz()`

Bad: `valueIsXyz()`

* Use AssertJ for assertions in preference to JUnit’s Assert class.

To help future-proof the code (avoiding libraries that may be deprecated in the near future), avoid assertions from the `org.assertj.core.api.Java6Assertions` class. Java6 in the name is a concerning signal

## 4.2.5 Miscellaneous

* When creating loggers it should be the first declaration in the class with:

`private static final Logger LOG = getLogger();`

* Ternary operators are acceptable when they make the code clearer but should never be nested

* Avoid TODO comments. Log a ticket instead

* Specify Gradle dependency versions in `versions.gradle`

* Don't use two or more blank lines in a row

# 5 Logging

Logging is important for understanding what EthSigner is doing at any given time (for example, progress while synchronizing) and investigating defects. During development, add logging to aid in these cases.

## 5.1 Log Messages

Make log messages:
* Not so frequent they are overwhelming in the log output
* At the appropriate level
* Detailed enough to understand what actually happened. For example:

`Insufficient validators. Expected 10 but found 4`

* As succinct as possible while still being clear.

* Bad: `Insufficient validators. Expected 10 but got: [address, address, address, address, address, address]`

## 5.2 Log Levels

* _Trace_

Extremely detailed view showing the execution flow. Likely only useful to developers

* _Debug_

Information that is diagnostically helpful to a wider group than just developers (for example, sysadmins)

* _Info_

Generally useful information to log (for example, service start/stop, configuration assumptions). Default logging level

* _Warn_

Anything that can potentially cause application oddities but from which EthSigner automatically recovers

* _Error_

Any error which is fatal to the operation, but not EthSigner itself (for example, missing data)

* _Fatal_

An error that forces a shutdown of EthSigner

0 comments on commit c5160bb

Please sign in to comment.