[Pulp-dev] [noissue] considered harmful
dalley at redhat.com
Wed Nov 6 17:50:26 UTC 2019
Agreed, I would also rather keep [noissue]. That is a good example of when
it is useful. We can and should pay more attention to when it's being used
in the PR review process, though.
On Wed, Nov 6, 2019 at 12:26 PM Tatiana Tereshchenko <ttereshc at redhat.com>
> I'm leaning towards keeping [noissue] and have it a responsibility of PR
> reviewer/merger to evaluate if it's a proper use or an issue is needed.
> Alternatively, can we block a merge of PR if there is no issue? at github
> level, not in travis? so tests run but you can only merge it "using your
> admin privileges" by pressing a very red button :)
> One good case for noissue is all the release PRs I think. So if we let
> tests run regardless of the issue specified in the commit and only block
> merge, it's probably fine to press the red button for the release PRs only.
> On Wed, Nov 6, 2019 at 3:55 PM Matthias Dellweg <dellweg at atix.de> wrote:
>> Being a regular user of noissue, i think it is ok to keep it, if the
>> reviewer can tell the contributer at some time that the change is
>> effecting something worth noting in the changelog. Sometimes, what
>> starts as a very small change evolves into something bigger.
>> Sometimes you might even want to show some code without knowing whether
>> there is an issue at all. Removing this loophole would prevent those
>> code paths from ever running in the ci.
>> I'd vote for not removing the [noissue] functionality, but instead keep
>> an open eye on it's usage, and in case request that a ticket be opened.
>> On Wed, 6 Nov 2019 08:52:14 -0500
>> David Davis <daviddavis at redhat.com> wrote:
>> > When we added commit validation to our CI, we created a loophole that
>> > would allow small changes like typo fixes to not have a redmine issue
>> > or changelog entry. By having '[noissue]' in the commit message,
>> > users could bypass our commit requirements. However, this loophole is
>> > not being used as intended--it's being used for all sorts of changes
>> > from new features to bug fixes.
>> > This is going to be a major problem for users and plugin writers when
>> > we release 3.0 and they start to upgrade from release to release.
>> > These [noissue] changes won't being reflected in our changelog which
>> > invalidates the motivation behind having a changelog.
>> > My first inkling is to remove the '[noissue]' functionality. Other
>> > projects like Katello and foreman already do this. If you don't want
>> > your change reflected in the changelog, you should be creating .misc
>> > entries for these changes in the CHANGES folder. The downside to this
>> > is every change will require a changelog and issue, and this creates
>> > extra steps for devs and contributors.
>> > Another option is to make our check smarter by incorporating some
>> > other condition(s) or metric(s). For example, perhaps we could allow
>> > [noissue] changes only to certain directories like our tests
>> > directory. The problem here is getting the parameters right. What
>> > sorts of changes changes should we allow to be [noissue] ones? I've
>> > thought about this a bit and couldn't think of a good set of criteria.
>> > Thoughts?
>> > David
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
> Pulp-dev mailing list
> Pulp-dev at redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Pulp-dev