Adding check which determines whether or not there are existing commits which are unsigned. The check is determined by the no-dco label.

Signed-off-by: Martin Dekov (VMware) [email protected]

Adding succesful/failure check when unsigned commits are present in the PR

Description

Adding check which is successful by default and when no-dco label is applied the check fails, when the commits are signed the check is again successful

I think the allow merge if all checks pass would be turned on in the GitHub UI. I have red something about required status checks or something

Motivation and Context

• [x] I have raised an issue to propose this change (required)

Fixes #95

Also suggestions on the text, the content of the titles body etc. Would be appreciated!

How Has This Been Tested?

Tested in repository here

Note: I have local minikube instance exported with ngrok so ping me if the robot does not work

Verbose diagram:

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [ ] I have added tests to cover my changes.
• [x] All new and existing tests passed.

Derek already supports manipulation of Labels. The GitHub API for Milestones is very similar. Milestones are used to group issues that belong together as a feature or a deliverable. See here for the distinctions: https://guides.github.com/features/issues/

Expected Behaviour

Derek set milestone: 4.8 should add the named milestone to the current issue Derek remove milestone: 4.8 should accordingly remove the milestone

Current Behaviour

Milestones aren't supported at all

Possible Solution

Add code analogous to the Label handling.

Context

Subsurface is starting to use Milestones to group issues that we want to address for upcoming releases and it would be very useful if more people were able to set/remove milestones from issues, without being Collaborators on the project

Your Environment

• [x] You're using the hosted Derek service

or

• [ ] You host your own OpenFaaS cluster with Derek installed

me and Martin collaborated on this change over a couple of Zoom meetings. please, let me know if this needs any changes. /cc @rgee0 @alexellis

Description

Track the environment variable use_slash_trigger and make it possible to use / as the command trigger that the bot uses instead of "Derek ".

• Update unit tests in commentHandler_test.go.
• Update README.md

Signed-off-by: Martin Dekov (VMware) [email protected] Signed-off-by: Lubomir I. Ivanov (VMware) [email protected]

Motivation and Context

• [x] I have raised an issue to propose this change (required) https://github.com/alexellis/derek/issues/63

How Has This Been Tested?

unit tests only on my side. @mrtind and @ivanayov can possibly do an e2e if they have the setup.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

This checks if the branch a pull request is raised from is non-master. Validates, as well, if a pull request is raised against the default branch. If the validation is not successful, the pull request labels are applied: "review/target-branch" if the target branch is not the default and "review/source-branch" if the source branch is named "master". The source branch validation leaves a warning comment, suggesting to raise a new pull request from a named branch

Signed-off-by: Ivana Yovcheva (VMware) [email protected]

Raised in issue #17

Adding messages option to the derek bot in order to apply tempaltes or boilerplate things that we write all the time.

Signed-off-by: Martin Dekov (VMware) [email protected]

Description

Extending the comments handler, by passing the derek config file (.DEREK.yml), in order to take the messages field from it and pass it to the executing function which sends the message to the Issue.

Motivation and Context

• [x] I have raised an issue to propose this change (required)

Closes #109

How Has This Been Tested?

Manually multi line literal value of the message:

Note in order for this to work, the string literal should be placed just below the | like so:

message:
  - name: slack
    value: |
            --
            Join Slack to connect with the community
            https://docs.openfaas.com

If we go like this, this will lead to parsing error

message:
  - name: slack
    value: |
         --
         Join Slack to connect with the community
         https://docs.openfaas.com

Manually single line value of message:

Whole content of .DEREK.yml:

Image is here martindekov/derek:0.0.7, in order to test augment your .DEREK.yml file like pointed in the picture above and swap the images.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [x] I have added tests to cover my changes. 50%
• [x] All new and existing tests passed.

This is a suggestion to help with first impressions and helping new contributors to a project/repo.

To help users identify and give help to new contributors to a repository Derek could add a label such as derek/first-time or derek/new-contributor

This metadata may be represented in the metadata already of a GitHub push/PR - https://developer.github.com/v3/activity/events/types/#pushevent

Alex

Description

this PR adds support for the optional redirect filed in .DEREK.yaml to allow jumping away from this config and loading another. the change is non-breaking for existing users.

Motivation and Context

• [x] I have raised an issue to propose this change (required)

spoke to @alexellis about this at KubeCon. :) the idea is to allow multiple repos to use the same config file.

https://github.com/alexellis/derek/issues/58

How Has This Been Tested?

pushed a test config to a github fork and added a temporary test in permissionsHandler_test.go (not included) that fetches the config from a redirected URL. then did a go fmt, golint and go test -v on the changes.

the best way to test this is to pull this branch and add this in the yaml:

redirect: <some-config-url>

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

Expected Behaviour

There are often behaviors demonstrated by new contributors or specific questions to which we need to answer in almost the same, verbatim way:

• Join Slack here to chat more https://...
• Sign off your single commit using this command
• Sign off those 5 commits using this command
• Oh you're doing performance testing? You can't do that in a VM on your Mac, try this docs page: https://
• CI is broken, please check the logs
• This needs a rebase before it can be merged

Current Behaviour

Mixed results, repetition for maintainers and contributors.

Possible Solution

A command like: Derek topic: sign-off / Derek help topic: slack

Automating the sign-off message could be spammy, especially if someone has a number of commits and the message is sent each time. So rather than automatic, hands-off - a "semi-automatic" way of expanding snippets would be ideal.

We could keep these in .DEREK.yml such as:

topics:
  name: slack-join
  alias:
   - slack
  message: |
  Thank for your interest in OpenFaaS. If you'd like to continue the discussion
  or join our regular Zoom meetings, then please sign-up at https:// .... 

Derek topic: slack

Expected Behaviour

I visited CompareTheMarket today who have a use-case for Derek for delegation of how people label issues without admin access to inner-source repos.

They want Derek to work on their private repos.

Current Behaviour

Derek reads from a CDN for all repos, but with the private ones there is no CDN.

Possible Solution

Find out if the event came from a private repo by using the metadata of the event. If private, grab .DEREK.yml from the GitHub API If not, then go ahead and fallback to the CDN

Steps to Reproduce (for bugs)

1. Create private repo (this is free on GH now)
2. Install your dev instance of Derek
3. Push code and see nothing happen

This commit adds contributing_url field to the .DEREK.yml file to point to contribution guide.

Signed-off-by: Vivek Singh [email protected]

Description

Motivation and Context

• [x] I have raised an issue to propose this change (required) Fixes: #87

How Has This Been Tested?

Test result and process added in comments below.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

Description

Switches from dep to use go modules.

Motivation and Context

By switching to go modules it would bring dependency management to align with go ecosystem. Fixes #160

• [x] I have raised an issue to propose this change (required)

How Has This Been Tested?

make build test succeeded.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [ ] I have added tests to cover my changes.
• [x] All new and existing tests passed.

Currently in draft as need to consider the effect in generation of release notes for a pre-release. As proposed, this would be cumulative over a number of pre-releases as prerelease is only consider when finding the previous release, so each pre-release will be considered against the previous full release.

Signed-off-by: Richard Gee [email protected]

Description

Initially the release notes were being generated from the current tagged release (n) and the n-1 release, irrespective of whether n-1 was a full release. This change causes getWorkingReleases() to ignore any previous releases where PreRelease is set to true. The processing beyond this determination will continue to consider all commits between the two identified points, resulting in a fuller set of changes for a full release where Pre-Releases have occurred in between.

Motivation and Context

• [x] I have raised an issue to propose this change (required)

Closes #184

How Has This Been Tested?

TBC

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [x] Feature Enhancement

Checklist:

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

Signed-off-by: Richard Gee [email protected]

Description

The intended mode of operation for the Issue template headings was for the local to over-ride the remote. This is different to the other fields, such as maintainers.

This change adds the test to highlight the issue and addresses the issue in MergeDerekRepoConfigs.

Motivation and Context

A recent issue (https://github.com/alexellis/go-execute/issues/10) indirectly highlighted that the merging of local and remote configs was producing a superset which included duplicate headings

How Has This Been Tested?

Wrote the test, and ran it:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^Test_mergeDerekRepoConfigs_UseLocalHeadings$ github.com/alexellis/derek/types

--- FAIL: Test_mergeDerekRepoConfigs_UseLocalHeadings (0.00s)
    /Users/rgee0/go/src/github.com/alexellis/derek/types/merge_test.go:102: RequiredInIssues want [#2], but got [#1 #2 #2]
FAIL
FAIL    github.com/alexellis/derek/types    0.229s
FAIL

Note the got set is a merge of the remote and local so includes #2 twice.

Amended MergeDerekRepoConfigs() so that if required_in_issue is set locally then those values will take precedence. Re-ran the test:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^Test_mergeDerekRepoConfigs_UseLocalHeadings$ github.com/alexellis/derek/types

ok  	github.com/alexellis/derek/types	0.233s

Ran a build using the Dockerfile.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

Expected Behaviour

Derek should compare new releases against the latest official release rather than against the latest release which can be a pre-release.

Current Behaviour

When Derek generates the release notes he compares the last two releases regardless of whether they are pre-releases or not. This makes hard to read the changes from two official releases if there has been some release candidates.

Possible Solution

Compare new releases against the latest official release.

Steps to Reproduce (for bugs)

1. Create an official release (1.0.0)
2. Create a pre-release (1.1.0-rc.1)
3. Create an official release (1.1.0)
4. The generated notes will have no changes because its being compared against 1.1.0-rc.1 instead of 1.0.0

Context

I was trying to create a new release from a release candidate but now it's hard to read the release notes of the major release because if I want to read all the changes I have to go through all pre-releases to know what have change

2020/11/01 11:25:33 Duration: 0.008617 seconds
2020/11/01 11:25:34 stderr: 2020/11/01 11:25:34 Release [openfaas-incubator/ingress-operator:0.6.4] start: 0001-01-01 00:00:00 +0000 UTC	end: "0001-01-01 00:00:00 +0000 UTC"
2020/11/01 11:25:34 Release notes: "Changelog for :\n\nCommits\n\nGenerated by [Derek](https://github.com/alexellis/derek/)\n"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x6d2092]

goroutine 1 [running]:
github.com/alexellis/derek/handler.updateRelease(0xc0004128c0, 0x0, 0xc0000b4f20, 0x12, 0xc00014a000, 0x10, 0x0, 0x0, 0xc00019b5c0, 0x54, ...)
	/go/src/github.com/alexellis/derek/handler/release_handler.go:204 +0x62
github.com/alexellis/derek/handler.updateReleaseNotes(0xc0004128c0, 0xc0000b4f20, 0x12, 0xc00014a000, 0x10, 0xc000131fb0, 0x5, 0x0, 0xc0004128c0)
	/go/src/github.com/alexellis/derek/handler/release_handler.go:108 +0xb65
github.com/alexellis/derek/handler.(*UpdatingReleaseHandler).Handle(0xc0000aa400, 0xc000083610, 0xc0000ace60, 0xc000148000, 0xc0000c86e0, 0xc0000ee080, 0xc0003ba4c0, 0xde70b95193631570)
	/go/src/github.com/alexellis/derek/handler/release_handler.go:43 +0x116
main.handleEvent(0xc0000160f4, 0x7, 0xc0000fc000, 0x24f3, 0x3e00, 0xc0000a0120, 0x28, 0xc000102000, 0x68f, 0xc00001602f, ...)
	/go/src/github.com/alexellis/derek/main.go:103 +0x18af
main.main()
	/go/src/github.com/alexellis/derek/main.go:62 +0x1e4
2020/11/01 11:25:34 Success=false, Error=exit status 2
2020/11/01 11:25:34 Out=
2020/11/01 11:27:56 Forking fprocess.
2020/11/01 11:27:56 Query  
2020/11/01 11:27:56 Path  /
2020/11/01 11:27:58 stderr: 2020/11/01 11:27:58 Release [openfaas-incubator/ingress-operator:0.6.4] start: 2020-11-01 11:24:54 +0000 UTC	end: "2020-06-18 14:54:05 +0000 UTC"
2020/11/01 11:27:58 Release notes: "Changelog for 0.6.4:\n* PR #35 Use the ingress domain for the tls secret by @LucasRoesler\n\nCommits\ndf2332dfa1bc46f41bfbdb6d3066d104e3f584ac Use the ingress domain for the tls secret by @LucasRoesler\nf627c24ccb91e21065c1e62ef986743a63e85426 Fix example, move REST example to docs by @alexellis\na4c4ee1ed250a1da6c613ea76f6799da2dd4d234 Update REST API mapping by @alexellis\nd93fe76e30ef1f65b94f1fa32b503f4cca8b9695 Update examples for REST by @alexellis\ne7a6aae212eb150ddc012671f25439ade274304a Add async example by @alexellis\n\nGenerated by [Derek](https://github.com/alexellis/derek/)\n"
2020/11/01 11:27:58 Duration: 2.295807 seconds
2020/11/01 11:27:59 Forking fprocess.
2020/11/01 11:27:59 Query  
2020/11/01 11:27:59 Path  /
2020/11/01 11:27:59 Duration: 0.002734 seconds

When I re-tried the web hook for the release, it worked the second time -> https://github.com/openfaas-incubator/ingress-operator/releases/tag/0.6.4

Inconsistent error:

exit status 1
time="2020-07-24T10:05:39Z" level=fatal msg="Error getting commits for PR 44\nGET https://api.github.com/repos/martindekov/push2/pulls/44/commits: 401 Bad credentials []"

This error is here: https://github.com/alexellis/derek/blob/16d7143de95fbc1516be7377b7388995a78b19c0/handler/pullrequest_handler.go#L242

And the function is: https://github.com/alexellis/derek/blob/16d7143de95fbc1516be7377b7388995a78b19c0/handler/pullrequest_handler.go#L235

Things I went through while trying to make the error consistent and it didn't:

• [x] Using the same context across all Derek's code for the pull logic as the OAuth2 object has description that the client is valid as long as the context with which it was created is used
• [x] Check rate limits as per description https://docs.github.com/en/rest/reference/rate-limit#understanding-your-rate-limit-status since the DCO feature increases the number of API calls.

Expected Behaviour

Every request for the commits has the same behavior on error or success.

Current Behaviour

Requesting the commits in a PR is inconsistent and might fail.

Possible Solution

In this case we can re-try the request. Possibly if the problem is in the access token we can revisit how we request the token, or fix the error flow of the token.

Steps to Reproduce (for bugs)

1. Run derek
2. Start sending requests by opening PRs and adding commits with and without signature
3. Check that sometimes this error is present and commits cannot be fetched leading to the dco label and checks not being applied properly

Context

This can potentially lead to inconsistency when checking for DCO as if the error is present and derek will fail to list the commits and recognize if there is unsigned one.

Your Environment

• [ ] You're using the hosted Derek service

or

• [x] You host your own OpenFaaS cluster with Derek installed

• Docker version docker version (e.g. Docker 17.0.05 ): N/A
• Are you using Docker Swarm or Kubernetes (FaaS-netes)? N/A
• Operating System and version (e.g. Linux, Windows, MacOS): N/A

If there's a merge commit pushed to a PR can we add a label and post a message to the PR from Derek saying that Merge Commits are not allowed, and provide info on how to remove it, and rebase.

Can we detect easily merge commits? Is it the format of the commit message or are they different?

Expected Behaviour

A PR with a merge commit pushed to it should get a label from derek and a message posted about why and how to fix it. 2 scenarios

1. A PR is raised and there is already a merge commit
2. A Merge Commit is pushed to the PR

Should this be optional? Configured in the yaml? What about for repos that dont have the config set? Do we enable or disable by default?

Current Behaviour

No feature

Possible Solution

Add a check into Derek that fulfils the 2x scenarios if enabled.