IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Docs: Improve guides for contributing (#19575)

Browse Source 
* Move style guides into contribute directory

* Move contribution guides into contribute directory

* Refactor CONTRIBUTING.md

* Clean up docs/README.md

* Update reference to style guide and minor formatting fixes

* Apply suggestions from code review

Co-Authored-By: gotjosh <josue.abreu@gmail.com>

* Update CONTRIBUTING.md

Co-Authored-By: gotjosh <josue.abreu@gmail.com>
This commit is contained in:
Marcus Olsson
2019-10-03 14:13:58 +02:00
committed by GitHub
parent 27ddd2d60f
commit 2fb301ccaf
14 changed files with 189 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
pkg/README.md
View File

@@ -1,35 +1,43 @@
# Grafana backend codebase
The code [styleguide](STYLEGUIDE.md) and brief description of the [architecture](ARCHITECTURE.md)
The code [styleguide](/contribute/style-guides/backend.md) and brief description of the [architecture](ARCHITECTURE.md)
# On going refactorings.
These issues are not something we want to address all at once but something we will improve over time. Since Grafana is released at a regular schedule the prefer approuch is to do this in batches. Not only is it easier to review, it also reduces the risk of conflicts when cherry-picking fixes from master to release branches. Changes that spawn multiple locations are therefore prefered in the end of the release cycle since we make fewer patch releases in the end of the cycle.
# Ongoing refactorings
These issues are not something we want to address all at once but something we will improve over time. Since Grafana is released at a regular schedule the preferred approach is to do this in batches. Not only is it easier to review, but it also reduces the risk of conflicts when cherry-picking fixes from master to release branches. Please try to submit changes that span multiple locations at the end of the release cycle. We prefer waiting until the end because we make fewer patch releases at the end of the release cycle, so there are fewer opportunities for complications.
## Global state
Global state makes testing and debugging software harder and its something we want to avoid when possible.
Unfortunately, there is quite a lot of global state in Grafana. The way we want to migrate away from this
is to use the `inject` package to wire up all dependencies either in `pkg/cmd/grafana-server/main.go` or
self registering using `registry.RegisterService` ex https://github.com/grafana/grafana/blob/master/pkg/services/cleanup/cleanup.go#L25
## Reduce the use of the init() function
Should only be used to register services/implementations.
## Settings refactoring
The plan is to move all settings to from package level vars in settings package to the [setting.Cfg](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/setting/setting.go#L210) struct. To access the settings services/components can inject this setting.Cfg struct.
[Cfg struct](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/setting/setting.go#L210)
[Injection example](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/services/cleanup/cleanup.go#L20)
## Reduce the use of Goconvey
We want to migrated away from using Goconvey and use stdlib testing as its the most common approuch in the GO community and we think it will make it easier for new contributors. Read more about how we want to write tests in the [ARCHITECTURE.MD](ARCHITECTURE.md#Testing) docs.
## Sqlstore refactoring
The sqlstore handlers all use a global xorm engine variable. This should be refactored to use the Sqlstore instance.
## Avoid global HTTP Handler functions
HTTP handlers should be refactored to so the handler methods are on the HttpServer instance or a more detailed handler struct. E.g (AuthHandler). This way they get access to HttpServer service dependencies (& Cfg object) and can avoid global state
## Date comparison
Newly introduced date columns in the database should be stored as epochs if date comparison is required. This permits to have a unifed approach for comparing dates against all the supported databases instead of handling seperately each one of them. In addition to this, by comparing epochs error pruning transformations from/to other time zones are no more needed.
# Dependency management

27
pkg/STYLEGUIDE.md
View File

@@ -1,27 +0,0 @@
# Backend style guide
Grafanas backend has been developed for a long time with a mix of code styles.
This style guide is a guide for how we want to write Go code in the future. Generally, we want to follow the style guides used in Go [Code Review Comments](https://code.google.com/p/go-wiki/wiki/CodeReviewComments) and Peter Bourgon's [Go: Best Practices for Production Environments](http://peter.bourgon.org/go-in-production/#formatting-and-style)
## Linting and formatting
We enforce strict `gofmt` formating and use some linters on our codebase. You can lint the codebase with <akefile -
```bash
$ make lint-go
```
We use [revive](https://github.com/mgechev/revive) as a go linter, and do enforce our [custom config](https://github.com/grafana/grafana/blob/master/conf/revive.toml) for it.
The end goal is to follow the golint. And the approuch for reachin that goal is to lint all parts of the codebase that we are currently working on and enable stricter linting for more areas as we go.
## Testing
We use GoConvey for BDD/scenario based testing. Which we think is useful for testing certain chain or interactions. Ex https://github.com/grafana/grafana/blob/master/pkg/services/auth/auth_token_test.go
We value clean & readable code that is loosely coupled and covered by unit tests. This makes it easier to collaborate and maintain the code. In the sqlstore package we do database operations in tests and while some might say that's not suited for unit tests. We think they are fast enough and provide a lot of value.
For new tests its preferred to use standard library testing.
### Mocks/Stubs
As a general rule of thumb we try to override/replace functions/methods when mocks/stubs are needed. One common task is the need of overriding time (`time.Now()`). See usage of `getTime` variable in [code](https://github.com/grafana/grafana/blob/52c39904120fb0b98494b961be67bb47574245b1/pkg/services/auth/auth_token.go#L22) and in [test](https://github.com/grafana/grafana/blob/52c39904120fb0b98494b961be67bb47574245b1/pkg/services/auth/auth_token_test.go#L23-L26) as an example.
When you need to stub/mock an interface you can implement a struct that allows you to override methods on a test-by-test basis. See [stub](https://github.com/grafana/grafana/blob/52c39904120fb0b98494b961be67bb47574245b1/pkg/services/auth/testing.go) and [example usage](https://github.com/grafana/grafana/blob/52c39904120fb0b98494b961be67bb47574245b1/pkg/middleware/middleware_test.go#L153-L180).