Comment filter

[sbesson]

This PR is a proposal to address #214. See the issue for more context about the application of the PR

Current status and limitations

This PR works under the current assumptions:

• the default filter semantics used for the ci-master merge builds is unchanged, i.e. all PRs considered for inclusion should be opened by a public member of the organization or labelled as include and neither labelled as breaking nor labelled as exclude
• developers want to have the ability to label their PRs via comments and have them deployed in isolation from the main CI

A limitation of the current implementation is that the comment labelling is tightly coupled to the users filters. This implies that for a command like scc merge -Dnone -Ilabel, PR comments of type --label
will not be treated as labels.

Implementation

This proposal redefines the whitelisting mechanism for labelling PRs by comment. With this change, comments made by public members of the organization comments are always treated as labels independently of the set of filters passed to the command.

In term of use case, assuming a PR has been opened against a repository and commented by a public member of the organization with a --label command, with the following command:

scc merge master --info -Dnone -Ilabel

should not be listed as a candidate with the previous release of scc but should be listed with this set of changes.

Additionally, the test_merge.testIncludeComment has been extended to cover the semantics with various versions of the merge command.

/cc @aleksandra-tarkowska

[joshmoore]

... comments from public members of the organization are always treated as labels

This amounts to a workaround for the GitHub-permissions system. For all our current uses of labels, this sounds ideal.

[atarkowska]

https://10.0.51.154:8443/job/OMERO-push/2/console I am getting:

+ /home/omero/.local/bin/scc merge develop -Dnone -Ithumbnails --no-ask --reset --update-gitmodules -S success-only --push develop/merge/trigger/thumbnails
2017-02-14 23:14:42,424 [   scc.merge] INFO  Merging Pull Request(s) based on develop
2017-02-14 23:14:42,425 [   scc.merge] INFO  Including Pull Request(s) labelled as thumbnails
2017-02-14 23:14:42,425 [   scc.merge] INFO  Excluding Pull Request(s) without successful status
2017-02-14 23:18:09,609 [   scc.merge] INFO  Repository: openmicroscopy/openmicroscopy
2017-02-14 23:18:09,609 [   scc.merge] INFO  Excluded PRs:
... (all prs)
2017-02-14 23:22:11,974 [   scc.merge] INFO  Already up-to-date.
2017-02-14 23:22:11,974 [   scc.merge] INFO  
2017-02-14 23:22:11,974 [   scc.merge] INFO  Merged PRs:
2017-02-14 23:22:11,975 [   scc.merge] INFO    # PR 5081 aleksandra-tarkowska 'get thumbails'
2017-02-14 23:22:11,975 [   scc.merge] INFO  
2017-02-14 23:22:11,975 [   scc.merge] INFO  Repository: ome/scripts
2017-02-14 23:22:11,975 [   scc.merge] INFO  Already up-to-date.
2017-02-14 23:22:11,975 [   scc.merge] INFO  
2017-02-14 23:18:09,611 [   scc.merge] INFO  
Traceback (most recent call last):
  File "/home/omero/.local/lib/python2.7/site-packages/scc/main.py", line 76, in entry_point
    (UpdateSubmodules.NAME, UpdateSubmodules),
  File "/home/omero/.local/lib/python2.7/site-packages/yaclifw/framework.py", line 197, in main
    ns.func(ns)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 2952, in __call__
    self.push(args, self.main_repo)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 1987, in push
    main_repo.rpush(branch_name, remote, force=True)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 1836, in rpush
    self.push_branch(branch_name, remote=full_remote, force=force)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 107, in wrapper
    error = check_exception_message(e)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 99, in wrapper
    return func(*args, **kwargs)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 1152, in push_branch
    self.call("git", "push", "-f", remote, name)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 1018, in call
    return self.wrap_call(self.debugWrap, *command, **kwargs)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 1043, in wrap_call
    raise Exception("rc=%s" % rc)
Exception: rc=1
Build step 'Execute shell' marked build as failure
Finished: FAILURE

even pr is labelled -Dnone -Ilabel

[sbesson]

@aleksandra-tarkowska: after reviewing the output, I know what went wrong with the command. This is a known limitation of the Git branching mechanism independent of this PR.

Git branches are serialized as files under .git/refs/heads and branch names containing slashes will create additional folders. It is thus not possible to have two coexisting local branches called foo and foo/bar (see also references in https://coderwall.com/p/qkofma/a-caution-about-git-branch-names-with-s). This limitation is the main reason for our removal the master branch at initialization of the scc forks in order to create our master/merge/daily integration branches later.

In your job above, you tried to push to develop/merge/trigger/thumbnails which conflicts with the existing develop/merge/trigger remote branch. Can you try develop/merge/thumbnails instead?

[atarkowska]

true, all good :)

[sbesson]

Discussed with @aleksandra-tarkowska earlier today. Merging this and listing #216 for review. Once both PRs are included, I propose to cut a 0.7.0 release of scc.