Skip to content

Conversation

@gichiba
Copy link
Contributor

@gichiba gichiba commented May 24, 2019

This PR tracks all improvements to documentation following the merge of #614

  • updated instructions for running the reputation miner, including the reputation visualizer
  • improved 'get started' directions
  • improved extensions examples
  • an API section (blocked by the lack of a natspec parser) As Ryan pointed out, this was already solved in Generate documentation for interface contracts #622
  • updated network inheritance diagram (which includes domainRoles) this was too detailed so instead I did an architecture level diagram of the network design. The inheritance model is not really that useful on paper, plus it can get very brittle to maintain. Devs would normally go to the codebase to look at details like this.

@ryanchristo
Copy link
Contributor

ryanchristo commented May 25, 2019

an API section (blocked by the lack of a natspec parser)

This is built using a custom script in #622. The natspec comments in the interface contracts could use some punctuation fixes including the use of back ticks when referring to a _variable but the script is accurately parsing natspec comments without the need for an external natspec parser.

I'd like to propose that this branch remain in existence permanently, and all documentation changes be pushed here.

I think it would be best to keep the standard Gitflow Workflow, merging feature branches such as this into the develop branch and not breaking the pattern by adding an additional permanent branch.

When we are ready to publish changes to documentation, they should be aligned with a new release, therefore we would merge the develop branch into the master branch. If hotfixes need to be made to the documentation, a feature branch can merge those changes directly into master.

When we are ready to merge develop into master for a new release, we would first need to merge master into develop so that the develop branch includes the hotfixes we merged to master. This is a standard workflow that I think we should maintain.

We can then turn off the circleci builds to this branch and save their servers from frivolous flops.

As @elenadimitrova mentioned here in #612, you can skip CircleCI builds by adding [skip ci] to the end of the commit message.

@gichiba
Copy link
Contributor Author

gichiba commented May 27, 2019

Hmmm, yeah ok that seems to make sense, although I for some reason have doubts that the [skip ci] flag stops all of the builds from running. But I guess that's a very minor point to a much more important practice.

Okay, closing this and will open up a new feature-specific docs branch for the next round.

@gichiba gichiba closed this May 27, 2019
@elenadimitrova
Copy link
Contributor

I am going to reopen this PR for the purposes of updating the points Griffin made in the improvements list above. It won't "remain in existence permanently" and it will be merged around the imminent go-live. The current level of documentation (even with the lovely parsed interface docs) is not sufficient for a developer to be productively using the network. There is a lot of know-how that's just not captured.


### Test coverage
### Documentation-only contributions
If changes only affect files in the `docs/` directory, suspend CI builds by adding `[skip ci]` or `[ci skip]` to your commit message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"suspend" it too strong a word here. Maybe "skip"?

If you intend to work with `glider-rc.1` on the Görli testnet, proceed with installation below, skipping the "local development and testing" section.


### Installation
Copy link
Contributor

@elenadimitrova elenadimitrova May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is an almost exact duplicate of the repo readme which is why it goes out of date so often. I'd like to remove it altogether.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to butcher this section before @gichiba and @ryanchristo agree?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section could be improved upon and trimmed down a bit but I also think it would be best if a developer was able to get started without needing to leave the page (moving from the docs to the readme and back to the docs).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to fight very very hard against any duplication of installation instructions and dependencies. If you want to keep and maintain it here, I will remove it from the readme.

The Colony Network is a large set of contracts that together define how all colonies operate and interact with people and other smart contracts on the Ethereum blockchain.

Colony is designed to be modular, upgradable, and eternally backwards-compatible. This is achieved through a sophisticated contract architecture that requires a bit of exposition. It is recommended that any developer seeking to understand the Colony Network solidity implementation first read the [Colony White Paper](https://colony.io/whitepaper/), or at the very least, the [White Paper TL;DR](/colonynetwork/whitepaper-tldr-colony/).
Colony is designed to be modular, upgradable, and eternally backwards-compatible. This is achieved through a sophisticated contract architecture that requires a bit of exposition. It is recommended that any developer seeking to understand the Colony Network solidity implementation first read the [Colony White Paper](https://colony.io/whitepaper.pdf), or at the very least, the [White Paper TL;DR](/colonynetwork/whitepaper-tldr-colony/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elenadimitrova elenadimitrova force-pushed the documentation branch 2 times, most recently from f2419c1 to b6a60fd Compare May 30, 2019 12:40
Data structures, enums, constants and events are declared in a dedicated `*DataTypes.sol` contract, e.g. `ColonyDataTypes.sol`.

For clarity all storage variables are held separately in a `*Storage.sol` contract, e.g. `ColonyStorage.sol`. Storage variable declaration ordering is crucial to be maintained correctly as that ordering cannot change during contract upgrages, due to the [contract upgrade mechanism](/colonynetwork/docs-the-delegate-proxy-pattern/) we use.
For clarity, all storage variables are held separately in a `*Storage.sol` contract, e.g. `ColonyStorage.sol`. Storage variable declaration ordering is crucial to be maintained; network upgrades and recovery depend on a consistent and clear storage layout. All variabes are commented with slot numbers to support developers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

network upgrades and recovery depend on a consistent and clear storage layout
This is insufficient to convey the full meaning.
"ordering cannot change during contract upgrades" <- this is what it's all about. If you insert a new storage variable amongst existing ones for example, it'll break the upgrade even if the storage layout is clear.

Skill `254` has children: `[]`

Currently this role grants permission for no functions. It is a placeholder for the dispute resolution system, to be implemented in later releases.
If a user with "Admininstration" authority in domain `2` wants to finalize a payment in domain `5`, they would call:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope you approve of me changing the example function, @elenadimitrova. I think it's a little confusing to make the first example of permissioned functions a function that alters permissions 😛

@elenadimitrova elenadimitrova merged commit 9b15079 into develop Jun 6, 2019
@elenadimitrova elenadimitrova deleted the documentation branch June 6, 2019 18:34
@gichiba gichiba mentioned this pull request Jun 7, 2019
@kronosapiens kronosapiens added this to the Sprint 27 milestone Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants