You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by Justin Leet <ju...@gmail.com> on 2018/01/02 16:42:56 UTC

Re: [DISCUSS] Removing Markdown files from rat exclusion

The PR is merged into master, and all relevant PRs have a comment noting
that adding the header is required.

As a reminder, this means Apache headers are required on all markdown files
and this will be enforced by rat.

On Sat, Dec 30, 2017 at 8:33 AM, Justin Leet <ju...@gmail.com> wrote:

> I've updated the PR to add the header to a new MD file that went in.
>
> I've also commented on all PRs that I saw that would potentially be
> problematic were they to go into master if they weren't merged first.
>
> Once the updated PR gets the +1's reaffirmed, it will be merged into
> master and Markdown headers will be enforced properly going forwad.
>
> On Sun, Dec 24, 2017 at 8:09 PM, Justin Leet <ju...@gmail.com>
> wrote:
>
>> I'm gonna let this percolate until Wednesday or so, assuming conversation
>> doesn't reach a natural tipping point.  I'm inclined to agree with Nick,
>> but I also don't want to resolve anything in a way that even potentially
>> causes master problems until at least after Christmas has a chance to
>> settle down for people.  At that point, assuming current course, I'll take
>> a real run through of the PRs (and leave comments as appropriate, before
>> merging.
>>
>> Obviously if anyone has suggestions or alternatives, still feel
>> encouraged to respond.
>>
>> On Sat, Dec 23, 2017 at 11:17 AM, Nick Allen <ni...@nickallen.org> wrote:
>>
>>> > This would result in master breaking (although it's a pretty easy fix).
>>>
>>> I am not concerned and don't think we need to wait on merging PR #883.
>>>
>>> Can you add a comment to each of the PRs that you identified?  We can
>>> make
>>> sure that each gets merged with master before they go in.
>>>
>>>
>>>
>>> On Sat, Dec 23, 2017 at 11:08 AM, Justin Leet <ju...@gmail.com>
>>> wrote:
>>>
>>> > I have a PR currently out (https://github.com/apache/metron/pull/883)
>>> that
>>> > removes the rat exclusion on Markdown files. There was a discuss thread
>>> > awhile back about adding the header and removing the exclusion where
>>> it was
>>> > agreed that we should do this to meet Apache requirements.
>>> Unfortunately,
>>> > it didn't get any follow on.
>>> >
>>> > Right now the PR has two +1s, but it could potentially be problematic
>>> with
>>> > existing PRs.
>>> >
>>> > Any PR that meets two conditions could potentially be problematic
>>> > 1. It adds a new Markdown file
>>> > 2. Travis was run before the exclusion PR was merged.
>>> >
>>> > This is because whoever does the merge might not realize that master
>>> should
>>> > be merged in and the markdown file updated with the Apache header.
>>> This
>>> > would result in master breaking (although it's a pretty easy fix).
>>> >
>>> > Are we okay with merging this now/soon, or do we want to take
>>> additional
>>> > steps to ensure we don't run into issues? If we want, I can run
>>> through the
>>> > PRs and add comments before merging.  Is this sufficient to at least
>>> > mitigate the most obvious problems?
>>> >
>>> > I took a very quick glance through some of the most recent PRs and
>>> only two
>>> > really stood out to me (although I'm sure there are older ones that are
>>> > still being worked on or looked at)
>>> >
>>> > METRON-1380 https://github.com/apache/metron/pull/882 - Adds a new
>>> > markdown
>>> > file, but Travis failed. If it gets fixed before this PR is merged we
>>> could
>>> > run into the problem
>>> > METRON-1351 https://github.com/apache/metron/pull/868 - Adds a new
>>> > markdown
>>> > file and Travis succeeded. This would break master if merged as-is
>>> after my
>>> > PR.
>>> >
>>>
>>
>>
>

Re: [DISCUSS] Removing Markdown files from rat exclusion

Posted by Matt Foley <ma...@apache.org>.
Bravo.

On 1/2/18, 8:43 AM, "Justin Leet" <ju...@gmail.com> wrote:

    The PR is merged into master, and all relevant PRs have a comment noting
    that adding the header is required.
    
    As a reminder, this means Apache headers are required on all markdown files
    and this will be enforced by rat.
    
    On Sat, Dec 30, 2017 at 8:33 AM, Justin Leet <ju...@gmail.com> wrote:
    
    > I've updated the PR to add the header to a new MD file that went in.
    >
    > I've also commented on all PRs that I saw that would potentially be
    > problematic were they to go into master if they weren't merged first.
    >
    > Once the updated PR gets the +1's reaffirmed, it will be merged into
    > master and Markdown headers will be enforced properly going forwad.
    >
    > On Sun, Dec 24, 2017 at 8:09 PM, Justin Leet <ju...@gmail.com>
    > wrote:
    >
    >> I'm gonna let this percolate until Wednesday or so, assuming conversation
    >> doesn't reach a natural tipping point.  I'm inclined to agree with Nick,
    >> but I also don't want to resolve anything in a way that even potentially
    >> causes master problems until at least after Christmas has a chance to
    >> settle down for people.  At that point, assuming current course, I'll take
    >> a real run through of the PRs (and leave comments as appropriate, before
    >> merging.
    >>
    >> Obviously if anyone has suggestions or alternatives, still feel
    >> encouraged to respond.
    >>
    >> On Sat, Dec 23, 2017 at 11:17 AM, Nick Allen <ni...@nickallen.org> wrote:
    >>
    >>> > This would result in master breaking (although it's a pretty easy fix).
    >>>
    >>> I am not concerned and don't think we need to wait on merging PR #883.
    >>>
    >>> Can you add a comment to each of the PRs that you identified?  We can
    >>> make
    >>> sure that each gets merged with master before they go in.
    >>>
    >>>
    >>>
    >>> On Sat, Dec 23, 2017 at 11:08 AM, Justin Leet <ju...@gmail.com>
    >>> wrote:
    >>>
    >>> > I have a PR currently out (https://github.com/apache/metron/pull/883)
    >>> that
    >>> > removes the rat exclusion on Markdown files. There was a discuss thread
    >>> > awhile back about adding the header and removing the exclusion where
    >>> it was
    >>> > agreed that we should do this to meet Apache requirements.
    >>> Unfortunately,
    >>> > it didn't get any follow on.
    >>> >
    >>> > Right now the PR has two +1s, but it could potentially be problematic
    >>> with
    >>> > existing PRs.
    >>> >
    >>> > Any PR that meets two conditions could potentially be problematic
    >>> > 1. It adds a new Markdown file
    >>> > 2. Travis was run before the exclusion PR was merged.
    >>> >
    >>> > This is because whoever does the merge might not realize that master
    >>> should
    >>> > be merged in and the markdown file updated with the Apache header.
    >>> This
    >>> > would result in master breaking (although it's a pretty easy fix).
    >>> >
    >>> > Are we okay with merging this now/soon, or do we want to take
    >>> additional
    >>> > steps to ensure we don't run into issues? If we want, I can run
    >>> through the
    >>> > PRs and add comments before merging.  Is this sufficient to at least
    >>> > mitigate the most obvious problems?
    >>> >
    >>> > I took a very quick glance through some of the most recent PRs and
    >>> only two
    >>> > really stood out to me (although I'm sure there are older ones that are
    >>> > still being worked on or looked at)
    >>> >
    >>> > METRON-1380 https://github.com/apache/metron/pull/882 - Adds a new
    >>> > markdown
    >>> > file, but Travis failed. If it gets fixed before this PR is merged we
    >>> could
    >>> > run into the problem
    >>> > METRON-1351 https://github.com/apache/metron/pull/868 - Adds a new
    >>> > markdown
    >>> > file and Travis succeeded. This would break master if merged as-is
    >>> after my
    >>> > PR.
    >>> >
    >>>
    >>
    >>
    >