[Pulp-dev] [noissue] considered harmful

[Dana Walker]

+1 reviewers watch out for it

Dana Walker

She / Her / Hers

Software Engineer, Pulp Project

Red Hat <https://www.redhat.com>

dawalker at redhat.com
<https://www.redhat.com>



On Wed, Nov 6, 2019 at 2:04 PM Ina Panova <ipanova at redhat.com> wrote:

>
> +1 to keep [noissue] and reviewer keeps an eye whether an opened issue is
> needed.
>
> --------
> Regards,
>
> Ina Panova
> Senior Software Engineer| Pulp| Red Hat Inc.
>
> "Do not go where the path may lead,
>  go instead where there is no path and leave a trail."
>
>
> On Wed, Nov 6, 2019 at 6:51 PM Daniel Alley <dalley at redhat.com> wrote:
>
>> 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>
>> wrote:
>>
>>> 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
>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>> _______________________________________________
>>> Pulp-dev mailing list
>>> Pulp-dev at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>
>> _______________________________________________
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20191106/c191ec44/attachment.htm>