Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-03-18 23:02:30 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2022-03-18 23:02:30 +0300
commit41fe97390ceddf945f3d967b8fdb3de4c66b7dea (patch)
tree9c8d89a8624828992f06d892cd2f43818ff5dcc8 /doc/development/contributing
parent0804d2dc31052fb45a1efecedc8e06ce9bc32862 (diff)
Add latest changes from gitlab-org/gitlab@14-9-stable-eev14.9.0-rc42
Diffstat (limited to 'doc/development/contributing')
-rw-r--r--doc/development/contributing/design.md2
-rw-r--r--doc/development/contributing/issue_workflow.md16
-rw-r--r--doc/development/contributing/merge_request_workflow.md5
-rw-r--r--doc/development/contributing/style_guides.md25
-rw-r--r--doc/development/contributing/verify/index.md236
5 files changed, 254 insertions, 30 deletions
diff --git a/doc/development/contributing/design.md b/doc/development/contributing/design.md
index efa8d4b0c41..463a7ee0e0b 100644
--- a/doc/development/contributing/design.md
+++ b/doc/development/contributing/design.md
@@ -35,7 +35,7 @@ Check these aspects both when _designing_ and _reviewing_ UI changes.
- Use clear and consistent [terminology](https://design.gitlab.com/content/terminology/).
- Check grammar and spelling.
- Consider help content and follow its [guidelines](https://design.gitlab.com/usability/helping-users/).
-- Request review from the [appropriate Technical Writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers),
+- Request review from the [appropriate Technical Writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments),
indicating any specific files or lines they should review, and how to preview
or understand the location/context of the text from the user's perspective.
diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md
index ad8403d242c..4db686b9b1e 100644
--- a/doc/development/contributing/issue_workflow.md
+++ b/doc/development/contributing/issue_workflow.md
@@ -45,7 +45,7 @@ scheduling into milestones. Labeling is a task for everyone. (For some projects,
Most issues will have labels for at least one of the following:
-- Type. For example: `~"type::feature"`, `~"type::bug"`, or `~"type::tooling"`.
+- Type. For example: `~"type::feature"`, `~"type::bug"`, or `~"type::maintenance"`.
- Stage. For example: `~"devops::plan"` or `~"devops::create"`.
- Group. For example: `~"group::source code"`, `~"group::knowledge"`, or `~"group::editor"`.
- Category. For example: `~"Category:Code Analytics"`, `~"Category:DevOps Reports"`, or `~"Category:Templates"`.
@@ -72,19 +72,7 @@ labels, you can _always_ add the type, stage, group, and often the category/feat
Type labels are very important. They define what kind of issue this is. Every
issue should have one and only one.
-The current type labels are:
-
-- `~"type::feature"`
- - `~"feature::addition"`
- - `~"feature::enhancement"`
-- `~"type::maintenance"`
-- `~"type::bug"`
-- `~"type::tooling"`
- - `~"tooling::pipelines"`
- - `~"tooling::workflow"`
-- `~"support request"`
-- `~meta`
-- `~documentation`
+The current type labels are [available in the handbook](https://about.gitlab.com/handbook/engineering/metrics/#work-type-classification)
A number of type labels have a priority assigned to them, which automatically
makes them float to the top, depending on their importance.
diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md
index 02148f2a717..a9b4d13ab06 100644
--- a/doc/development/contributing/merge_request_workflow.md
+++ b/doc/development/contributing/merge_request_workflow.md
@@ -81,7 +81,7 @@ request is as follows:
1. If your MR touches code that executes shell commands, reads or opens files, or
handles paths to files on disk, make sure it adheres to the
[shell command guidelines](../shell_commands.md)
-1. If your code needs to handle file storage, see the [uploads documentation](../uploads.md).
+1. If your code needs to handle file storage, see the [uploads documentation](../uploads/index.md).
1. If your merge request adds one or more migrations, make sure to execute all
migrations on a fresh database before the MR is reviewed. If the review leads
to large changes in the MR, execute the migrations again once the review is complete.
@@ -264,8 +264,11 @@ requirements.
1. Peer member testing is optional but recommended when the risk of a change is high. This includes when the changes are [far-reaching](https://about.gitlab.com/handbook/engineering/development/#reducing-the-impact-of-far-reaching-work) or are for [components critical for security](../code_review.md#security).
1. Regressions and bugs are covered with tests that reduce the risk of the issue happening
again.
+1. Code affected by a feature flag is covered by [automated tests with the feature flag enabled and disabled](../feature_flags/index.md#feature-flags-in-tests), or both
+ states are tested as part of peer member testing or as part of the rollout plan.
1. [Performance guidelines](../merge_request_performance_guidelines.md) have been followed.
1. [Secure coding guidelines](https://gitlab.com/gitlab-com/gl-security/security-guidelines) have been followed.
+1. [Application and rate limit guidelines](../merge_request_application_and_rate_limit_guidelines.md) have been followed.
1. [Documented](../documentation/index.md) in the `/doc` directory.
1. [Changelog entry added](../changelog.md), if necessary.
1. Reviewed by relevant reviewers, and all concerns are addressed for Availability, Regressions, and Security. Documentation reviews should take place as soon as possible, but they should not block a merge request.
diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md
index da926005466..7a4ebbdbadf 100644
--- a/doc/development/contributing/style_guides.md
+++ b/doc/development/contributing/style_guides.md
@@ -159,25 +159,22 @@ When the number of RuboCop exceptions exceed the default [`exclude-limit` of 15]
we may want to resolve exceptions over multiple commits. To minimize confusion,
we should track our progress through the exception list.
-When auto-generating the `.rubocop_todo.yml` exception list for a particular Cop,
-and more than 15 files are affected, we should add the exception list to
-a different file in the directory `.rubocop_todo/`. For example, the configuration for the cop
-`Gitlab/NamespacedClass` is in `.rubocop_todo/gitlab/namespaced_class.yml`.
-
-This ensures that our list isn't mistakenly removed by another auto generation of
-the `.rubocop_todo.yml`. This also allows us greater visibility into the exceptions
-which are currently being resolved.
-
-One way to generate the initial list is to run the Rake task `rubocop:todo:generate`:
+The preferred way to [generate the initial list or a list for specific RuboCop rules](../rake_tasks.md#generate-initial-rubocop-todo-list)
+is to run the Rake task `rubocop:todo:generate`:
```shell
+# Initial list
bundle exec rake rubocop:todo:generate
+
+# List for specific RuboCop rules
+bundle exec rake 'rubocop:todo:generate[Gitlab/NamespacedClass,Lint/Syntax]'
```
-You can then move the list from the freshly generated `.rubocop_todo.yml` for the Cop being actively
-resolved and place it in the directory `.rubocop_todo/`. In this scenario, do not commit
-auto-generated changes to the `.rubocop_todo.yml`, as an `exclude limit` that is higher than 15
-makes the `.rubocop_todo.yml` hard to parse.
+This Rake task creates or updates the exception list in `.rubocop_todo/`. For
+example, the configuration for the RuboCop rule `Gitlab/NamespacedClass` is
+located in `.rubocop_todo/gitlab/namespaced_class.yml`.
+
+Make sure to commit any changes in `.rubocop_todo/` after running the Rake task.
### Reveal existing RuboCop exceptions
diff --git a/doc/development/contributing/verify/index.md b/doc/development/contributing/verify/index.md
new file mode 100644
index 00000000000..a2bb0eca733
--- /dev/null
+++ b/doc/development/contributing/verify/index.md
@@ -0,0 +1,236 @@
+---
+type: reference, dev
+stage: none
+group: Verify
+---
+
+# Contribute to Verify stage codebase
+
+## What are we working on in Verify?
+
+Verify stage is working on a comprehensive Continuous Integration platform
+integrated into the GitLab product. Our goal is to empower our users to make
+great technical and business decisions, by delivering a fast, reliable, secure
+platform that verifies assumptions that our users make, and check them against
+the criteria defined in CI/CD configuration. They could be unit tests, end-to-end
+tests, benchmarking, performance validation, code coverage enforcement, and so on.
+
+Feedback delivered by GitLab CI/CD makes it possible for our users to make well
+informed decisions about technological and business choices they need to make
+to succeed. Why is Continuous Integration a mission critical product?
+
+GitLab CI/CD is our platform to deliver feedback to our users and customers.
+
+They contribute their continuous integration configuration files
+`.gitlab-ci.yml` to describe the questions they want to get answers for. Each
+time someone pushes a commit or triggers a pipeline we need to find answers for
+very important questions that have been asked in CI/CD configuration.
+
+Failing to answer these questions or, what might be even worse, providing false
+answers, might result in a user making a wrong decision. Such wrong decisions
+can have very severe consequences.
+
+## Core principles of our CI/CD platform
+
+Data produced by the platform should be:
+
+1. Accurate.
+1. Durable.
+1. Accessible.
+
+The platform itself should be:
+
+1. Reliable.
+1. Secure.
+1. Deterministic.
+1. Trustworthy.
+1. Fast.
+1. Simple.
+
+Since the inception of GitLab CI/CD, we have lived by these principles,
+and they serve us and our users well. Some examples of these principles are that:
+
+- The feedback delivered by GitLab CI/CD and data produced by the platform should be accurate.
+ If a job fails and we notify a user that it was successful, it can have severe negative consequences.
+- Feedback needs to be available when a user needs it and data can not disappear unexpectedly when engineers need it.
+- It all doesn’t matter if the platform is not secure and we
+are leaking credentials or secrets.
+- When a user provides a set of preconditions in a form of CI/CD configuration, the result should be deterministic each time a pipeline runs, because otherwise the platform might not be trustworthy.
+- If it is fast, simple to use and has a great UX it will serve our users well.
+
+## Building things in Verify
+
+### Measure before you optimize, and make data-informed decisions
+
+It is very difficult to optimize something that you can not measure. How would you
+know if you succeeded, or how significant the success was? If you are working on
+a performance or reliability improvement, make sure that you measure things before
+you optimize them.
+
+The best way to measure stuff is to add a Prometheus metric. Counters, gauges, and
+histograms are great ways to quickly get approximated results. Unfortunately this
+is not the best way to measure tail latency. Prometheus metrics, especially histograms,
+are usually approximations.
+
+If you have to measure tail latency, like how slow something could be or how
+large a request payload might be, consider adding custom application logs and
+always use structured logging.
+
+It's useful to use profiling and flamegraphs to understand what the code execution
+path truly looks like!
+
+### Strive for simple solutions, avoid clever solutions
+
+It is sometimes tempting to use a clever solution to deliver something more
+quickly. We want to avoid shipping clever code, because it is usually more
+difficult to understand and maintain in the long term. Instead, we want to
+focus on boring solutions that make it easier to evolve the codebase and keep the
+contribution barrier low. We want to find solutions that are as simple as
+possible.
+
+### Do not confuse boring solutions with easy solutions
+
+Boring solutions are sometimes confused with easy solutions. Very often the
+opposite is true. An easy solution might not be simple - for example, a complex
+new library can be included to add a very small functionality that otherwise
+could be implemented quickly - it is easier to include this library than to
+build this thing, but it would bring a lot of complexity into the product.
+
+On the other hand, it is also possible to over-engineer a solution when a simple,
+well tested, and well maintained library is available. In that case using the
+library might make sense. We recognize that we are constantly balancing simple
+and easy solutions, and that finding the right balance is important.
+
+### "Simple" is not mutually exclusive with "flexible"
+
+Building simple things does not mean that more advanced and flexible solutions
+will not be available. A good example here is an expanding complexity of
+writing `.gitlab-ci.yml` configuration. For example, you can use a simple
+method to define an environment name:
+
+```yaml
+deploy:
+ environment: production
+ script: cap deploy
+```
+
+But the `environment` keyword can be also expanded into another level of
+configuration that can offer more flexibility.
+
+```yaml
+deploy:
+ environment:
+ name: review/$CI_COMMIT_REF_SLUG
+ url: https://prod.example.com
+ script: cap deploy
+```
+
+This kind of approach shields new users from the complexities of the platform,
+but still allows them to go deeper if they need to. This approach can be
+applied to many other technical implementations.
+
+### Make things observable
+
+GitLab is a DevOps platform. We popularize DevOps because it helps companies
+be more efficient and achieve better results. One important component of
+DevOps culture is to take ownership over features and code that you are
+building. It is very difficult to do that when you don’t know how your features
+perform and behave in the production environment.
+
+This is why we want to make our features and code observable. It
+should be written in a way that an author can understand how well or how poorly
+the feature or code behaves in the production environment. We usually accomplish
+that by introducing the proper mix of Prometheus metrics and application
+loggers.
+
+**TODO** document when to use Prometheus metrics, when to use loggers. Write a
+few sentences about histograms and counters. Write a few sentences highlighting
+importance of metrics when doing incremental rollouts.
+
+### Protect customer data
+
+Making data produced by our CI/CD platform durable is important. We recognize that
+data generated in the CI/CD by users and customers is
+something important and we must protect it. This data is not only important
+because it can contain important information, we also do have compliance and
+auditing responsibilities.
+
+Therefore we must take extra care when we are writing migrations
+that permanently removes data from our database, or when we are define
+new retention policies.
+
+As a general rule, when you are writing code that is supposed to remove
+data from the database, file system, or object storage, you should get an extra pair
+of eyes on your changes. When you are defining a new retention policy, you
+should double check with PMs and EMs.
+
+### Get your changes reviewed
+
+When your merge request is ready for reviews you must assign
+reviewers and then maintainers. Depending on the complexity of a change, you
+might want to involve the people that know the most about the codebase area you are
+changing. We do have many domain experts in Verify and it is absolutely acceptable to
+ask them to review your code when you are not certain if a reviewer or
+maintainer assigned by the Reviewer Roulette has enough context about the
+change.
+
+The reviewer roulette offers useful suggestions, but as assigning the right
+reviewers is important it should not be done automatically every time. It might
+not make sense to assign someone who knows nothing about the area you are
+updating, because their feedback might be limited to code style and syntax.
+Depending on the complexity and impact of a change, assigning the right people
+to review your changes might be very important.
+
+If you don’t know who to assign, consult `git blame` or ask in the `#verify`
+Slack channel (GitLab team members only).
+
+### Incremental rollouts
+
+After your merge request is merged by a maintainer, it is time to release it to
+users and the wider community. We usually do this with feature flags.
+While not every merge request needs a feature flag, most merge
+requests in Verify should have feature flags. [**TODO** link to docs about what
+needs a feature flag and what doesn’t].
+
+If you already follow the advice on this page, you probably already have a
+few metrics and perhaps a few loggers added that make your new code observable
+in the production environment. You can now use these metrics to incrementally
+roll out your changes!
+
+A typical scenario involves enabling a few features in a few internal projects
+while observing your metrics or loggers. Be aware that there might be a
+small delay involved in ingesting logs in Elastic or Kibana. After you confirm
+the feature works well with internal projects you can start an
+incremental rollout for other projects.
+
+Avoid using "percent of time" incremental rollouts. These are error prone,
+especially when you are checking feature flags in a few places in the codebase
+and you have not memoized the result of a check in a single place.
+
+### Do not cause our Universe to implode
+
+During one of the first GitLab Contributes events we had a discussion about the importance
+of keeping CI/CD pipeline, stage, and job statuses accurate. We considered a hypothetical
+scenario relating to a software being built by one of our [early customers](https://about.gitlab.com/blog/2016/11/23/gitlab-adoption-growing-at-cern/)
+
+> What happens if software deployed to the [Large Hadron Collider (LHC)](https://en.wikipedia.org/wiki/Large_Hadron_Collider),
+> breaks because of a bug in GitLab CI/CD that showed that a pipeline
+> passed, but this data was not accurate and the software deployed was actually
+> invalid? A problem like this could cause the LHC to malfunction, which
+> could generate a new particle that would then cause the universe to implode.
+
+That would be quite an undesirable outcome of a small bug in GitLab CI/CD status
+processing. Please take extra care when you are working on CI/CD statuses,
+we don’t want to implode our Universe!
+
+This is an extreme and unlikely scenario, but presenting data that is not accurate
+can potentially cause a myriad of problems through the
+[butterfly effect](https://en.wikipedia.org/wiki/Butterfly_effect).
+There are much more likely scenarios that
+can have disastrous consequences. GitLab CI/CD is being used by companies
+building medical, aviation, and automotive software. Continuous Integration is
+a mission critical part of software engineering.
+
+When you are working on a subsystem for pipeline processing and transitioning
+CI/CD statuses, request an additional review from a domain expert and hold
+others accountable for doing the same.