chore(master): merge maint-0.9 (#92)

[mdonadoni]

Closes #90

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 3.78%. Comparing base (b0e3669) to head (4612f58).

❗ Current head 4612f58 differs from pull request most recent head 690dfc2. Consider uploading reports for the commit 690dfc2 to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##           master     #92      +/-   ##
=========================================
- Coverage    3.82%   3.78%   -0.05%     
=========================================
  Files           6       6              
  Lines         183     185       +2     
=========================================
  Hits            7       7              
- Misses        176     178       +2     

Files	Coverage Δ
reana_workflow_engine_snakemake/executor.py	0.00% <0.00%> (ø)

[tiborsimko]

Suggested changes:

Let's first detect three things about the current commit:

        commit_hash=$(echo "$line" | cut -d ' ' -f 1)
        commit_title=$(echo "$line" | cut -d ' ' -f 2-)
        commit_number_of_parents=$(git rev-list --parents "$commit_hash" -n1 | awk '{print NF-1}')

And then let's have some checking rules:

1. Is this ommit ending with the good PR number? -> OK

        if echo "$commit_title" | grep -qP "\(\#$pr\)$"; then
            true

2. Is this commit a release commit without a number? -> OK

        elif echo "$commit_title" | grep -qP "^chore\(.*\): release "; then
            true

3. Is this commit a merge commit?

• 3a) if it is of the type "chore(...): merge " then OK (and stop checking further commits);
• 3b) otherwise reply that merge commits are not allowed in feature branches (and continue checking further commits).

Something like the following (untested):

        if [ "$commit_number_of_parents" -gt 1 ]; then
            if echo "$commit_title" | grep -qP "^chore\(.*\): merge "; then
                break
            else
                echo "✖   Merge commits are not allowed in feature branches: $message"
                found=1
            fi
        fi

[mdonadoni]

I have implemented your suggestions, except that 3a/3b are now the first ones to be checked, as otherwise a merge commit whose title contains the correct PR number would not be considered an error even in a feature branch, given that it would pass check 1.

I have also changed a little bit the CI pipeline to consider commits from base..head instead of head~num_of_commits_in_pr..head, as the latter might include commits that are on the "wrong side" of the merge tree (that is, commits that are in base, which should not be considered as they are already merged)

[tiborsimko]

Cosmetic comment: if the merge check case is now being done first, then perhaps you can also invert the two following cases, in order to have:

        elif echo "$commit_title" | grep -qP "^chore\(.*\): release"; then
            true
        elif echo "$commit_title" | grep -qP "\(\#$pr\)$"; then
            true

In this way all the merge-like special cases are listed first, followed by the regular PR case, and the final error in case the PR case is not passing.

[tiborsimko]

Works nicely for merge commits 👍

Left a cosmetic comment about the order of cases in ./run-tests.sh --check-commitlint .