| Age | Commit message (Collapse) | Author |
|---|
|
Our `changes_size` Dangerfile currently uses the `added_files` and
`modified_files` methods to get a list of all files to check. However,
`modified_files` may return the old name of a renamed file. When this
occurs, the `diff` property for that file may be `nil`, causing the
danger job to error out with:
bundler: failed to load command: danger (/builds/gitlab-org/gitaly/_build/cache/ruby/ruby/3.0.0/bin/danger)
/builds/gitlab-org/gitaly/_build/cache/ruby/ruby/3.0.0/gems/danger-9.2.0/lib/danger/danger_core/plugins/dangerfile_git_plugin.rb:144:in `info_for_file': \e[31m (Danger::DSLError)
[!] Invalid `Dangerfile` file: \e[31m
[!] Invalid `Dangerfile` file: undefined method `[]' for nil:NilClass\e[0m
# from danger/rules/changes_size/Dangerfile:8
# -------------------------------------------
#
> info = git.info_for_file(file)
#
# -------------------------------------------
\e[0m
# from danger/rules/changes_size/Dangerfile:8
See [0] for full output.
A fix for this was attempted in d763b7b6d (Dangerfile: Fix segfault
when computing change size with renamed files, 2023-01-19), but in this
case `file` is `nil`, rather than `info`.
To prevent this from occurring, capture the list of renames and
remove the old names from the list of files to check. A previously
failing pipeline that now passes can be viewed at [1].
[0] https://gitlab.com/gitlab-org/gitaly/-/jobs/3764808135
[1] https://gitlab.com/gitlab-org/gitaly/-/jobs/3769648816
|
|
We're quite regularly seeing failures in our Dangerfile CI job that
looks like the following:
```
[!] Invalid `Dangerfile` file: \e[31m
[!] Invalid `Dangerfile` file: undefined method `[]' for nil:NilClass\e[0m
# from danger/rules/changes_size/Dangerfile:8
# -------------------------------------------
#
> git.info_for_file(file)[:insertions]
# end
# -------------------------------------------
\e[0m
# from danger/rules/changes_size/Dangerfile:8
# -------------------------------------------
# %r{\\Adoc/.*(\\.(md|png|gif|jpg))\\z} => :docs,
> %r{\\A(CONTRIBUTING|LICENSE|README|REVIEWING|STYLE)(\\.md)?\\z} => :docs,
#
# -------------------------------------------
```
This failure happens when we're trying to find out how many lines of
code have changed while filtering out some specific files like generated
Protobuf files.
Most notably, this failure seems to happen whenever the commit under
test contains renamed files. What probably happens is that it cannot
find the old name of the renamed file anymore, and consequentially it
will fail with a `nil` pointer exception.
Fix this bug by explicitly checking for the case where we haven't found
a specific file in Git.
|
|
|
|
Recently our Danger job has started failing with the following error
message:
```
[!] There was an error parsing `injected gems`: You cannot specify the same gem twice with different version requirements.
You specified: gitlab-dangerfiles (~> 3.1.0) and gitlab-dangerfiles (>= 0). Gem already added. Bundler cannot continue.
```
As it turns out, this breakage is caused by the upstream change 9e3eff4
(Don't require projects to contain Gemfile when using danger-review job,
2022-06-10) in the common pipeline configurations: if the CI job detects
that there is no Gemfile, it has now started to write one for us that
contains the `gitlab-dangerfiles` version. And because our Gemfile is
not contained in the root directory but instead in the `danger/` sub
directory we now end up with two Gemfiles.
Fix this issue by making use of the new feature: instead of carrying our
own Gemfile, we can now simply rely on the auto-generated Gemfile. While
this forces us to get rid of the Ruby cache which was keyed by our own
Gemfile, this is really not an issue at all: execution of the whole job
only takes about 40 seconds without the cache, which is fast enough.
|
|
This change adds a lightly adjusted version of the changes_size rule.
This version excludes the generated protobuf files *.pb.go and *_pb.rb.
|
|
|
|
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
This rule was introduced in
https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/releases/v2.1.0
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
The rule was introduced in
https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/releases/v2.9.0
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
Use features introduced in
https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/releases/v2.6.0
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
|
|
In our Dangerfile, we keep a list of team members, which is the perfect
kind of variable to grow stale. And of course it is stale given that
@jcaigitlab is missing from that list. Furthermore, its use is quite
limited: the only thing we use it for is to propose to add a
~group::gitaly label if the merge request author is part of the group.
Remove that group and the label-recommendation. While we could improve
this to just fetch the list of team members automagically, it doesn't
really feel worth.
|
|
Since we have upgraded gitlab-dangerfiles in 9f152be50 (danger: Update
gitlab-dangerfiles gem, 2021-12-07) we always see duplicate roulette
runs in merge requests. This is because the update also included a new
default roulette implementation, which implements roughly the same logic
as we have.
Remove our own roulette rule to avoid the duplication.
|
|
It's best practice for Danger rules to live in a separate "rules/"
directory. Let's move them there, which also allows us to use globs to
import all rules.
|
|
Update to version 2.6.1 so it has the latest rules that allow formatting
of MR titles as we use them. This was fixed in [1].
1. https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/merge_requests/69
|
|
Update Danger-related Gems to the latest versions. Most importantly,
this pulls in gitlab-dangerfiles v2.5.0, which relaxes rules when
parsing commit subjects to allow more characters before colons, like we
frequently use them in Gitaly.
|
|
|
|
Imports the Dangerfile code from GDK.
|
|
Since our goal is to remove the ruby sidecar from gitaly, we want a
separate and stable location to manage our danger dependencies.
|
|
Changelog trailers are processed case-sensitively by the API. This
updates Danger so it errors when using incorrect casing, such as
`changelog` instead of `Changelog`.
See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62915 for more
information.
|
|
Similar to https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62012,
this updates Gitaly's CI tooling for the new changelog workflow.
|
|
As I'm moving into a code contributor role again, I'd like to review
again too to build more knowledge of our codebase. This change suggests
zj-gitlab again to review MRs and thus I should slowly get more review
work to do.
|
|
This adds a configuration file for generating changelogs using the
GitLab API (done using Release Tools), as well as change Danger to
remind developers of these upcoming changes (similar to what we're doing
in gitlab-org/gitlab).
Omnibus, Pages and Helm are already using this new approach. Our current
plan is to also roll this out to remaining projects (GitLab Rails and
Gitaly) per May 22nd.
See https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1564 for
more information.
|
|
Prior to this change, it was hard to estimate how much MRs a person had
assigned for review. Also, there were no reviewers suggested if the MR
was assigned to yourself already.
This change adds a link to the suggested reviewer's MR dashboards. These
dashboard show the MRs assigned to them, but authored by someone
else. This should give an indication of how many reviews they have on
their plate already.
The reviewers suggestion will now also no longer disappear, and we use
the MR IID to seed the random number to suggest reviewers, which should
give the same names every time.
|
|
When a merge request has the documentation label it does not need Danger
to trigger a warning to create a changelog entry.
|
|
An MRs title is allowed to have a scope like "praefect:" as prefix,
which helps narrowing down which area of the code is being changed. The
rule is quite strict though and doesn't allow for numbers in the scope,
but given our recent trips into Git2Go land this case does come up quite
often now.
Make the rule more lax and allow numbers as part of the scope.
|
|
|
|
In the Git ecosystem, commit titles are commonly scoped to a given
subsystem to make immediately clear which parts of the system are being
changed (see e.g. this very commit's title). Using the same pattern for
merge requests is thus a natural urge for many used to this, but our
Danger rules will complain about such commit titles as their first
character is not an uppercase on.e
Let's improve the rule to allow optional (lower-cased) scopes.
|
|
|
|
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
When creating a changelog entry, there's a slightly award dance where
the changelog entry needs to contain a merge request id which is unknown
at the time of generation. This create the situation where either a
developer guesses the next MR ID or has to create the MR, update the
changelog entry, and push again.
This change lets @GitalyBot suggest the MR ID, which can be applied by
the developer. Squashing the commit is left to the developer to do.
|
|
A few minutes ago, I flipped the project setting that sets the 'Delete
source branch' tick by default when creating a new merge request. This
makes the danger check for the same thing irrelevant as the user chose
not to delete the branch if it's not set. Probably with good reason.
|
|
|
|
GitalyBot will try to automatically apply a milestone, and sorts the
milestones to apply based on the due date. Some milestones don't have a
due date. Notably, %Backlog doesn't have a due date.
While this might indicate a milestone isn't applied correctly on the
issues, the script should still work.
|
|
|
|
This would be a round robin way of getting a reviewer. Minimal
implementation, to see if it works.
|
|
Pavlo starts on Monday, and I'd like to prepare this change so I
don't forget.
|
|
|
|
Includes `@zj-gitlab` in the Gitaly team, and removes the Gitaly label
in favour of `devops::create` and `group::gitaly`.
|
|
|
|
If the issue this MR will close through either the commit messages or
the description have relevant labels, apply them on the MR too.
Closes https://gitlab.com/gitlab-org/gitaly/issues/1555
|
|
|
|
GitalyBot gets the issues the merge request closes and will detect the
first upcoming milestone and apply it.
This can be overridden by the user, and GitalyBot will not try again.
Another issue might be that if a MR slips, the milestone isn't updated.
|
|
Today I noticed that the labeling as applied previously aren't correct
for the throughput measurements GitLab does:
https://about.gitlab.com/handbook/engineering/management/throughput/#implementation
This change updates the labeling rules so from now on this will be
correctly applied. In the case of Gitaly performance is a feature and
not backstage as the point of Gitaly is improving performance.
En passant it fixes a small issue when no changelog entry is present.
|
|
Spotted on a MR where I was missing a MR ID, I removed a constant
which was required and used. By reintroducing the constant Danger should
work correctly again.
The docs had to link somewhere, so this change adds documentation too.
|
|
Using Danger a small rule book, @GitalyBot will apply labels and have a
few more rules on the changelog.
|
