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 2017/12/23 16:08:16 UTC
[DISCUSS] Removing Markdown files from rat exclusion
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.
>>> >
>>>
>>
>>
>
Re: [DISCUSS] Removing Markdown files from rat exclusion
Posted by Justin Leet <ju...@gmail.com>.
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 Justin Leet <ju...@gmail.com>.
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 Justin Leet <ju...@gmail.com>.
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 Nick Allen <ni...@nickallen.org>.
> 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.
>