Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-10449

Excluded regions do not exclude commits from changelog

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • subversion-plugin
    • None

      When using Excluded Regions to exclude certain parts of a repository from triggering a build (because it does not affect the build), the commits performed to those excluded regions are still listed in the changelog. If this is a feature and not a bug, it'd be great to at least have an option to exclude them from the changelog.

          [JENKINS-10449] Excluded regions do not exclude commits from changelog

          Jørgen Tjernø created issue -

          Eugene Leonov added a comment - - edited

          Of course it also relates to "Included Regions" field! Perhaps "Excluded Users" and "Excluded Commit Message" too, it's up to you.

          There is another reason to resolve this issue: JIRA Plugin writes comments to all issues from all commits in "Changes". I don't sure, perhaps Jira Plugin gets issue-list from commits itself (without using "changes" field). If so, I'll create separate issue with "jira" component. Could anybody answer me?

          Eugene Leonov added a comment - - edited Of course it also relates to "Included Regions" field! Perhaps "Excluded Users" and "Excluded Commit Message" too, it's up to you. There is another reason to resolve this issue: JIRA Plugin writes comments to all issues from all commits in "Changes". I don't sure, perhaps Jira Plugin gets issue-list from commits itself (without using "changes" field). If so, I'll create separate issue with "jira" component. Could anybody answer me?

          lhotari added a comment -

          This also affects the git plugin.

          lhotari added a comment - This also affects the git plugin.

          Brent Atkinson added a comment - - edited

          It seems like it should be a simple matter of iterating through the changes generated for the changelog, and filtering them based on the same rules for the detection check. However, this is more complex than it seems at first.

          The detection happens quite separately from the changelog generation. The detection happens in a private inner class in husdon.scm.SubversionSCM$SVNLogHandler.checkEntry, the handler is coupled to SVNKit's SVNLogEntry class. The changelog generation has its own LogEntry class, and there are a lot of private methods in SubversionSCM that it depends on, so you can't simply factor out the same code to filter them.

          Brent Atkinson added a comment - - edited It seems like it should be a simple matter of iterating through the changes generated for the changelog, and filtering them based on the same rules for the detection check. However, this is more complex than it seems at first. The detection happens quite separately from the changelog generation. The detection happens in a private inner class in husdon.scm.SubversionSCM$SVNLogHandler.checkEntry, the handler is coupled to SVNKit's SVNLogEntry class. The changelog generation has its own LogEntry class, and there are a lot of private methods in SubversionSCM that it depends on, so you can't simply factor out the same code to filter them.

          I ended up figuring out a way to cleanly unify the filtering. See https://github.com/jenkinsci/subversion-plugin/pull/27

          Brent Atkinson added a comment - I ended up figuring out a way to cleanly unify the filtering. See https://github.com/jenkinsci/subversion-plugin/pull/27

          kutzi added a comment -

          I'm not sure that this is a feature every user wants to have.
          I for my case would like to see the whole changelog even if not every commit would trigger a new build.
          So I think this should be configurable.

          BTW: if you're in a situation like these and need to exclude certain regions, it might be a signal that your subversion repository should be modularized.

          kutzi added a comment - I'm not sure that this is a feature every user wants to have. I for my case would like to see the whole changelog even if not every commit would trigger a new build. So I think this should be configurable. BTW: if you're in a situation like these and need to exclude certain regions, it might be a signal that your subversion repository should be modularized.

          I agree, that was my first reaction as well. If you're doing this systematically it will be an uphill battle and definitely suggests you're fighting your tooling. However, when considering the reasons to exclude parts of the tree from triggering a build, I couldn't generate reasonable arguments for including those changes in the changelog. If the commits will be noise for the builds, it seems to follow that they'll be noise in changelog as well. That's when I started looking at this differently and started working on a solution.

          Would you mind sharing your rationale? Why would you want to see the logs from excluded regions and not verify the project stability by building? Understanding the common reasons for using exclusions/inclusions might help steer others clear of expecting this.

          Brent Atkinson added a comment - I agree, that was my first reaction as well. If you're doing this systematically it will be an uphill battle and definitely suggests you're fighting your tooling. However, when considering the reasons to exclude parts of the tree from triggering a build, I couldn't generate reasonable arguments for including those changes in the changelog. If the commits will be noise for the builds, it seems to follow that they'll be noise in changelog as well. That's when I started looking at this differently and started working on a solution. Would you mind sharing your rationale? Why would you want to see the logs from excluded regions and not verify the project stability by building? Understanding the common reasons for using exclusions/inclusions might help steer others clear of expecting this.

          kutzi added a comment -

          One part of the reasoning is that these changes are actually checked out into the workspace, therefore I'd like to see them.
          Other part is that even if you think that the changes may not be relevant for the build, you may be wrong and suddenly the build starts failing without any changelog entry.

          Also I think that most of the time the additional changelog entries will do no harm, so I just would like to see them for completeness (see point 1

          kutzi added a comment - One part of the reasoning is that these changes are actually checked out into the workspace, therefore I'd like to see them. Other part is that even if you think that the changes may not be relevant for the build, you may be wrong and suddenly the build starts failing without any changelog entry. Also I think that most of the time the additional changelog entries will do no harm, so I just would like to see them for completeness (see point 1

          Thanks for explaining, I agree with your reasoning and it matches my initial thinking. My personal preference would be to never exclude and use proper sub-trees. However, assuming that there are valid reasons for exclusion, which is suggested by the fact it exists:

          • If it affects the build, it's a misconfiguration to exclude something that matters. You want to run verification on relevant changes.
          • If there are a lot of them, like developer docs needing locality to code, you don't want to be spammed with those changes
          • When excluding noisy edits that aren't functionality related. The full changelog may actually do more to hide changes in noise.
          • Ultimately, you want the misconfiguration fixed because you were wrong to exclude in the first place.

          I can see valid cases where the changes in the excluded area are particularly long and it's unlikely they'll be affecting anything, docs for example. I also can see wanting to have those edits omitted from the stream rather than spamming on the next build. It's the same reasoning, but a simulation of http://svnbook.red-bean.com/nightly/en/svn.advanced.sparsedirs.html

          Brent Atkinson added a comment - Thanks for explaining, I agree with your reasoning and it matches my initial thinking. My personal preference would be to never exclude and use proper sub-trees. However, assuming that there are valid reasons for exclusion, which is suggested by the fact it exists: If it affects the build, it's a misconfiguration to exclude something that matters. You want to run verification on relevant changes. If there are a lot of them, like developer docs needing locality to code, you don't want to be spammed with those changes When excluding noisy edits that aren't functionality related. The full changelog may actually do more to hide changes in noise. Ultimately, you want the misconfiguration fixed because you were wrong to exclude in the first place. I can see valid cases where the changes in the excluded area are particularly long and it's unlikely they'll be affecting anything, docs for example. I also can see wanting to have those edits omitted from the stream rather than spamming on the next build. It's the same reasoning, but a simulation of http://svnbook.red-bean.com/nightly/en/svn.advanced.sparsedirs.html

          kutzi added a comment -

          You don't have to convince me that there are lots of good use cases to exclude the changes from the changelog. I'd even say that these use cases are more relevant than my use cases.

          Unfortunately, to exclude the changes from the changelog is a backward incompatible change and should therefore (IMHO) be configurable.
          Who knows, maybe someone else has a perfectly reasonable usecase to not exclude the changes and he would be negatively affected by this change.

          kutzi added a comment - You don't have to convince me that there are lots of good use cases to exclude the changes from the changelog. I'd even say that these use cases are more relevant than my use cases. Unfortunately, to exclude the changes from the changelog is a backward incompatible change and should therefore (IMHO) be configurable. Who knows, maybe someone else has a perfectly reasonable usecase to not exclude the changes and he would be negatively affected by this change.

            Unassigned Unassigned
            jorgenpt Jørgen Tjernø
            Votes:
            6 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: