-
Notifications
You must be signed in to change notification settings - Fork 1
Added detailed Contributing guideline (#191) #233
base: experimental
Are you sure you want to change the base?
Conversation
a98a933
to
ba92cd1
Compare
|
||
Great if you want to contribute to this project! | ||
|
||
But we have to maintain this project and to do that easier everyone has to observe the follwing rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"we have"
that's not rly true, better would be "Our job as maintainers is to guarantee high code quality and standards, which makes it easier for other people, including us, to contribute to this project. Because of that everyone should follow certain rules while contributing to this project."
|
||
## Code Style | ||
|
||
The code is written like the official [Java Code Conventions](https://www.oracle.com/technetwork/java/codeconventions-150003.pdf). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
our code isn't written like some conventions - our conventions are somewhere similar to the oracle java code conventions
|
||
The code is written like the official [Java Code Conventions](https://www.oracle.com/technetwork/java/codeconventions-150003.pdf). | ||
|
||
Important is to give the methods and classes expressive and clear names to easily differentiate between them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use more titles, for example: ### Expressive method and class names
|
||
Important is to give the methods and classes expressive and clear names to easily differentiate between them. | ||
|
||
A command should be wrapped in a new line if it has more than 120 characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command?
"Statement" is the word u are searching for
|
||
A command should be wrapped in a new line if it has more than 120 characters. | ||
|
||
Tab characters have to be used instead of spaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"As the standard indentation for code we use tabs"
ControllerResult<SolutionModel> solutionResult = this.solutionController.getSolution(taskId); | ||
```` | ||
|
||
DTOs are named if they are are requestDTO ``dto``, if it is a responseDTO they are called `responseDTO`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicate "are" and the sentence is hard to understand bc of improper grammar
- controller<br> | ||
└ controller of endpoints | ||
- database<br> | ||
└ repositories of controllers and models of those repositories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing verb somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and repositories have nothing to do with controllers - just models. Remember: We are using MVC, which separates these layers
- database<br> | ||
└ repositories of controllers and models of those repositories | ||
- endpoint<br> | ||
└ endpoint classes and response/request DTOs (**D**ata**T**ransfer**O**bject) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DTOs are in a sub-package of "endpoint"
- endpoint<br> | ||
└ endpoint classes and response/request DTOs (**D**ata**T**ransfer**O**bject) | ||
- pubsub<br> | ||
└ pubsub classes and subscriber/publisher for the [PubSubSystem](https://github.com/E-Edu/concept/blob/master/documentation/dev/architecture/architecture.md#pubsub-cluster) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plural for "subscriber/publisher"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean exactly?
- pubsub<br> | ||
└ pubsub classes and subscriber/publisher for the [PubSubSystem](https://github.com/E-Edu/concept/blob/master/documentation/dev/architecture/architecture.md#pubsub-cluster) | ||
- spring<br> | ||
└ custom config files of spring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Spring related classes"
|
||
## Issues | ||
|
||
##### Any Bugs, Feature Request or Improvements? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use three instead of four
|
||
##### Any Bugs, Feature Request or Improvements? | ||
|
||
[Write us an issue!](https://github.com/E-Edu/task/issues/new/choose) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They won't write an issue to us, they create an issue
Closes #191