Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR Template & Updating Contributing guidelines #1121

Closed
wants to merge 36 commits into from

Conversation

lock9
Copy link
Contributor

@lock9 lock9 commented Sep 27, 2019

Close #1118

I tried to have different templates for Neo 3 and Neo 2 but it didn't work.
I also moved the contributing document to the .github folder.

@codecov-io
Copy link

codecov-io commented Sep 27, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@af2d821). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1121   +/-   ##
=========================================
  Coverage          ?   64.34%           
=========================================
  Files             ?      200           
  Lines             ?    13690           
  Branches          ?        0           
=========================================
  Hits              ?     8809           
  Misses            ?     4881           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af2d821...311df3c. Read the comment docs.

Co-Authored-By: Shargon <shargon@gmail.com>
shargon
shargon previously approved these changes Sep 30, 2019
@lock9
Copy link
Contributor Author

lock9 commented Oct 1, 2019

@neo-project/core one more approval please

shargon
shargon previously approved these changes Oct 15, 2019
@shargon
Copy link
Member

shargon commented Oct 18, 2019

@lock9

image

@lock9
Copy link
Contributor Author

lock9 commented Dec 3, 2019

@igormcoelho @vncoelho @erikzhang please help me approve this. If you want changes, please request changes.

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

@lock9, I will review it tomorrow. It is little bit late here.

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

@lock9, I will not approve this now because it involves also the CHANGELOG.md. Are we agreed that we are going to provide it? I am not sure.

@lock9
Copy link
Contributor Author

lock9 commented Dec 5, 2019

Hi @vncoelho, I removed the changelog file.
The contributing file has only been moved.

**Checklist**
- [ ] Did you add tests covering your changes?
- [ ] Did you run `dotnet format`?
- [ ] Did you update `CHANGELOG.md` with your changes?
Copy link
Member

Choose a reason for hiding this comment

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

There is no CHANGELOG.md now.

@lock9
Copy link
Contributor Author

lock9 commented Dec 6, 2019

@erikzhang Done

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

@lock9,I tried to commit directly or modify but I was not allowed:

I wrote the following modifications:

# Contributing to NEO
Neo is an open-source project and it depends on its contributors and constant community feedback to implement the features required for a smart economy. You are more than welcome to join us in the development of Neo.  

Read this document to understand how issues are organized and how you can start contributing.

*This document covers this repository only and does not include community repositories or repositories managed by NGD Shanghai and NGD Seattle.*

### Questions and Support
The issue list is open for any kind of discussions, questions and interaction with the collaborators of the project. In addition, it is also a good place for bug reports, possible features and optimizations. Do not hesitate to initiate any discussion. However, please, kindly take a look at possible similar keywords in order to try to find a possible connection with your idea.  

### dApp Development
This document does not relate to dApp development. If you are looking to build a dApp using Neo, please [start here](https://neo.org/dev).

### Contributing to open source projects
If you are new to open-source development, please [read here](https://opensource.guide/how-to-contribute/#opening-a-pull-request) how to submit your code.

## Developer Guidance
We try to have as few rules as possible,  just enough to keep the project organized:

1.  **Discuss before coding**. Proposals must be discussed before being implemented.  
Avoid implementing issues with the discussion tag.

2. **Tests during code review**. We expect reviewers to test the issue before approving or requesting changes.

3. **Wait for at least 2 reviews before merging**. Changes can be merged after 2 approvals, for Neo 3.x branch, and 3 approvals for Neo 2.x branch.

3. **Give time to other developers review an issue**. Even if the code has been approved, you should leave at least 24 hours for others to review it before merging the code.

4. **Create unit tests**. It is important that the developer includes basic unit tests so reviewers can test it.

5. **Task assignment**. If a developer wants to work in a specific issue, he may ask the team to assign it to himself. The proposer of an issue has priority in task assignment.

### Issues for beginners
If you are looking to start contributing to NEO, we suggest you start working on issues with ![](./images/cosmetic.png) or ![](./images/house-keeping.png) tags since they usually do not depend on extensive NEO platform knowledge. 

### Tags for Issues States

![Discussion](./images/discussion.png) Whenever someone posts a new feature request, the tag discussion is added. This means that there is no consensus if the feature should be implemented or not. Avoid creating PR to solve issues in this state since it may be completely discarded.

![Design](./images/solution-design.png) When a feature request is accepted by the team, but there is no consensus about the implementation, the issue is tagged with design. We recommend the team to agree in the solution design before anyone attempts to implement it, such as using text, UML, pseudo-code and proofs-of-concepts.
Note that PRs for issues in this state may also be discarded if the team disagree with the proposed solution.

![Ready-to-implement](./images/ready-to-implement.png) Once the team has agreed on feature and the proposed solution, the issue is tagged with ready-to-implement. When implementing it, please follow the solution accepted by the team.

### Tags for Issue Types

![Cosmetic](./images/cosmetic.png) Issues with the cosmetic tag are usually changes in code or documentation that improve user experience without affecting current functionality. These issues are recommended for beginners.

![Enhancement](./images/enhancement.png) Enhancements are platform changes that may affect performance, usability or add new features to existing modules. It is recommended that developers have previous knowledge in the platform to work in these improvements, specially in more complicated modules like the compiler, ledger and consensus.

![Feature](./images/new-feature.png) New features may include large changes in the code base. Some are complex, but some are not. So, a few issues with new-feature may be recommended for starters.

![Migration](./images/migration.png) Issues related to the migration from Neo 2 to Neo 3 are tagged with migration. These issues are usually the most complicated ones since they require a deep knowledge in both versions.

### Tags for Project Modules 
These tags do not necessarily represent each module at code level. Modules consensus and compiler are not recommended for beginners.

![Compiler](./images/compiler.png) Issues that are related or influence the behavior of our C# compiler. Note that the compiler itself is hosted in the [neo-devpack-dotnet](https://github.com/neo-project/neo-devpack-dotnet) repository.

![Consensus](./images/consensus.png) Consensus is used for reaching an agreement on which information will be published block by block. Changes to consensus are usually harder to make and test. 

![Ledger](./images/ledger.png) The ledger is our 'database', any changes in the way we store information or the data-structures have this tag.

![House-keeping](./images/house-keeping.png) 'Small' enhancements that need to be done in order to keep the project organised and ensure overall quality. These changes may be applied in any place in code, as long as they are small or do not alter current behavior/logic.

![Network-policy](./images/network-policy.png) Identify issues that affect the network-policy like fees, access list or other related issues. Voting may also be related to the network policy module.

![P2P](./images/p2p.png) This module includes peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP).

![RPC](./images/rpc.png) All HTTP communication is handled by the RPC module. This module usually provides support methods since the main communication protocol takes place at the p2p module.

![VM](./images/vm.png) New features that affect the Neo Virtual Machine or the Interop layer.

![SDK](./images/sdk.png) Neo provides an SDK to help developers to interact with the blockchain. Changes in this module must not impact other parts of the software. 

![Wallet](./images/wallet.png) Wallets are used to track funds and interact with the blockchain. Note that this module depends on a full node implementation (data stored on local disk).

@lock9
Copy link
Contributor Author

lock9 commented Dec 9, 2019

@vncoelho is it good? I can't see the order of the messages. When it is done, please remove the request for changes.

@vncoelho
Copy link
Member

vncoelho commented Dec 9, 2019

Can you apply my last message on the file? Just copy everything from the message and paste on the file.
You may see the diffs, they are minor but needed I think.

@vncoelho
Copy link
Member

vncoelho commented Dec 9, 2019

Or can you allow edit permission?

@vncoelho vncoelho changed the title PR Template PR Template & Updating Contributing guidelines Dec 9, 2019
@lock9
Copy link
Contributor Author

lock9 commented Dec 16, 2019

@vncoelho you have to send a PR to my repository

@vncoelho
Copy link
Member

They were already applied.

@erikzhang
Copy link
Member

image

@lock9 lock9 closed this Mar 13, 2020
@Tommo-L Tommo-L mentioned this pull request May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Pull Request Template
5 participants