Requesting permissions for several branch-source traits

[witokondoria]

Description

Three new plugins, implementing additional traits over branch-source plugins (currently github and bitbucket)

https://github.com/jenkinsci/branch-source-aged-refs-traits-plugin
https://github.com/jenkinsci/branch-source-commit-skip-traits-plugin
https://github.com/jenkinsci/branch-source-jira-validator-traits-plugin

https://issues.jenkins-ci.org/browse/HOSTING-414
https://issues.jenkins-ci.org/browse/HOSTING-417
https://issues.jenkins-ci.org/browse/HOSTING-428

branch-source-aged-refs-traits-plugin has a pending pull request (jenkinsci/scm-filter-aged-refs-plugin#1), blocked due a dependency, where the GAV will be changed to the one in this permission request.

Submitter checklist for changing permissions

Always

• Add link to plugin/component Git repository in description above

For a newly hosted plugin only

• Add link to resolved HOSTING issue in description above

For a new permissions file only

• Make sure the file is created in permissions/ directory
• artifactId (pom.xml) is used for name (permissions YAML file).
• groupId / artifactId (pom.xml) are correctly represented in path (permissions YAML file)
• Check that the file is named plugin-${artifactId}.yml for plugins

When adding new uploaders (this includes newly created permissions files)

• Use the Jenkins community (LDAP) account name in the YAML file, not the GitHub account name
• All newly added users have logged in to Artifactory at least once

[daniel-beck]

@stephenc AFAIUI this is the result of you rejecting new filters in branch source plugins. TBH I do not think rejecting additions such as this is the right approach and hope you reconsider.

This seems like very limited overhead if added to the branch source plugins (an additional entry in a dropdown somewhere?). Making people look for plugins implementing some (well generalizable) filters seems actively user-hostile. IIRC similar tools already use the tedious plugin management in Jenkins as "attack ads", why do we give them more ammunition?

[stephenc]

@daniel-beck everything we add to scm-api / branch-api / *-branch-source is exposed to ALL users and adds complexity to the users.

The decision that was taken is to support the core needs of the 80% of users in the core plugins and let all the other cases be handled via extension plugins.

If we add these traits to core, we add complexity to releasing the core plugins as the testing surface is larger. In fact I would very much like to remove the existing traits from scm-api and keep it a pure api plugin rather than a mix of api with some impl, but there were objections to the introduction of another plugin to the dependency tree.

The advantage of these small plugins is that they are much easier to test in isolation and users need only install the implementations they need.

That said, the names of these plugins I object to. These are not branch-source plugins but scm-api plugins. There is no dependency on branch-api (nor should there be), the names should be ending in scm-behaviour-impl to my mind so that user discovery is easier. The only reason we have "github-branch-source" and "bitbucket-branch-source" is because there already were "github" and "bitbucket" plugins that were solving a different use case. The correct name for those plugins should have been "github-scm" and "bitbucket-scm" given the existing name usage... in the cases where we have not had name conflicts, it is just a simple name such as "git" or "gitea" for the core scm-api plugin implementation. (the long term goal being moving the core scm functionality into scm-api)

Naming plugins with "branch-source" in the name is an anti-pattern that I would very much like to stamp out. Branch API is just one consumer of the SCM API.

Can we please revert and request a rename

[stephenc]

These are pure filters, plugin names should start or end with scm-filter, eg

github-aged-scm-filter Or scm-filter-by-age-github-impl

No branch source in the name please

[daniel-beck]

Can we please revert and request a rename

@witokondoria PTAL. Up to you.

@stephenc FYI #459 may be relevant as well.

[daniel-beck]

@stephenc

is exposed to ALL users and adds complexity to the users.

I acknowledge that here (AFAIUI):

very limited overhead if added to the branch source plugins (an additional entry in a dropdown somewhere?).

---

we add complexity to releasing the core plugins as the testing surface is larger […] advantage of these small plugins is that they are much easier to test in isolation and users need only install the implementations they need.

This is externalization of costs. It's nice that there's less to test in your plugin, but at the same time it's much more likely to break user instances due to incompatible changes. The average Jenkins instance today has 74 independently managed plugins (up from 52 just a year ago), and it's still growing rapidly. I'm not asking that plugins be combined needlessly because they do something similar. But these are clearly auxiliary features for branch sources with very little cost to have right there, as 'batteries included'.

[stephenc]

I'm not asking that plugins be combined needlessly because they do something similar. But these are clearly auxiliary features for branch sources with very little cost to have right there, as 'batteries included'.

Then very soon you end up with 100's of behaviours in every installation... now while that is preferable to 100's of checkboxes, it does not help the new user.

We could pool some of these filters into tiered plugins, but even there you end up exposing configuration options and complexity to many many users. The ones in #459 are probably generic enough that they could form the start of a std-scm-filter-impl plugin if we could find a couple more that are similarly generic... and then we could move the filters from scm-api into that std-scm-filter-impl (once we have plugin manager metadata to support those kinds of refactoring)

These ones are more complex. They need SCMSource specific code to identify the commit date on the branch as the details are not available from git's ls-remote so this functionality requires a checked out git repository or some git hosting provider... so there's always going to be a fork in the parent tree. Additionally these filters are somewhat specific...

• Filter PRs based on the JIRA ticket
• Filter Branches / PRs based on the last commit time (which for general users will have unexpected consequences)
• Filter Branches / PRs based on the last commit message (which will cause branches / PRs to become orphaned if the commit message contains some magic text, potentially deleting build history, and then restored again once the next commit contains some other magic text)

The general user does not want these behaviours unless they have very specific needs as they will play havoc with the build history... but there are some users who may want them... and as such I encourage they be published, just not with the current artifactIds

[daniel-beck]

@stephenc

Filter PRs based on the JIRA ticket

Right, hence generalizable: This looks like a "PR title/message regex filter". Either include or exclude on match, and it's general enough IMO.

Filter Branches / PRs based on the last commit message

Also general enough if configurable. Of course, the behavior you describe seems broken and should just skip building inapplicable commits, but otherwise this seems pretty generally useful too.

The criticisms of these plugins seem based on limitations of their current implementation. Those should just be fixed. My more general concerns about unnecessary plugin proliferation stands. It's a massive problem for users, and rejecting additions like these from 'core' plugins doesn't help.

[witokondoria]

My two cents:

the behavior you describe seems broken

The only way to skip a reference from being built is via the implementing th isExcluded method at the SCMHeadFilter or SCMHeadPreFilter. I now that as a side effect, the job and history could be eventually wiped and that might be unwanted for the 'Filter Branches / PRs based on the last commit message' trait. As eventually there wil lbe another commit, the job would be recreated (hence the lost history)

Wether the branch-api offered a isExcludedButKeep, I would be more that happy to use that extension

Filtering via PR title is meant to block the initial job creation, so the isExcludedButKeep doesnt seem to apply (even could be used)

Filtering via age is meant to completely block the job creation for old references, so isExcluded usage fits 100%

Regarding the PR title filter being generalizable, even it is conceptually, it has a jira-plugin dependency. Someone not using jira but willing to filter via a regexp, will earn a new plugin to its installed pool (and thats supposed to be unwanted).

I will go on with renaming the repos, GAVs and packagings and if the isExcludedButKeep method ever gets available, will switch to it.

[stephenc]

https://github.com/jenkinsci/scm-api-plugin/blob/master/docs/implementation.adoc#naming-your-plugin

[stephenc]

The commit message "filter" should actually be using https://issues.jenkins-ci.org/browse/JENKINS-45502 longer term. As such this would be more of a "decoration" rather than a "filter" so that one should be scm-trait-... or github-scm-trait-.../bitbucket-scm-trait-... per my naming recommendations because it is just because of the lack of an implementation of https://issues.jenkins-ci.org/browse/JENKINS-45502 that this is currently using a "hack" of being a filter.

The other two, age and PR title are certainly pure filter plugins and as such should be scm-filter-... and github-scm-filter-.../bitbucket-scm-filter-... per my naming recommendations