You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by dopheide-esnet <gi...@git.apache.org> on 2018/11/14 19:14:31 UTC
[GitHub] metron-bro-plugin-kafka pull request #18: Remove version from show-plugin bt...
GitHub user dopheide-esnet opened a pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
Remove version from show-plugin btest output in prep for Bro 2.7
## Contributor Comments
Update show-plugin.bro test to remove the version number from the baseline output.
With Bro 2.7, the version will be built from major.minor.patch, which conflicts with
the current major.minor in the baseline.
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron's Bro kafka writer plugin.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
### For code changes:
- [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
- [ ] Have you included steps or a guide to how the change may be verified and tested manually?
- [ ] Have you ensured that the full suite of tests and checks have been executed via:
```
bro-pkg test $GITHUB_USERNAME/metron-bro-plugin-kafka --version $BRANCH
```
- [ ] Have you written or updated unit tests and or integration tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building and running locally with Apache Metron's [Vagrant full-dev environment](https://github.com/apache/metron/tree/master/metron-deployment/development/centos6) or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dopheide-esnet/metron-bro-plugin-kafka topic/dopheide/2.7-btest-fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron-bro-plugin-kafka/pull/18.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18
----
commit 44bfd8b51f7fdda6a8ded6c6e998a9094adc0890
Author: Michael Dopheide <do...@...>
Date: 2018-11-14T19:13:17Z
Remove version from show-plugin btest output in prep for Bro 2.7
----
---
[GitHub] metron-bro-plugin-kafka issue #18: Remove version from show-plugin btest out...
Posted by dopheide-esnet <gi...@git.apache.org>.
Github user dopheide-esnet commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
I chatted with Siwek about it and this is merged into bro-aux as the
template for future users of init-plugin.
So it’s accepted in that regard as at least a short term solution to help
packages not break for people. I wouldn’t mind seeing something a bit more
elegant in the template that can actually check the correct plugin version
between Bro versions.
Dop
On Thu, Nov 15, 2018 at 2:22 PM JonZeolla <no...@github.com> wrote:
> So, I did put together a PR meant to handle this @
> metron-bro-plugin-kafka#8
>
> Do you happen to know if your approach is more generally accepted? It
> feels somewhat strange to say "version" without providing a version number
> at the very least, but I could see the benefits of having a show-plugin
> btest that works pre- and post- 2.7.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/metron-bro-plugin-kafka/pull/18#issuecomment-439177304>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AKtpitEwFHt40y56uAJMb9cpqhV3kYQgks5uvczzgaJpZM4YeZl->
> .
>
---
[GitHub] metron-bro-plugin-kafka issue #18: Remove version from show-plugin btest out...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
Sounds good, I'm inclined to accept but there is a little bit of housekeeping to take care of first. Can you please take a look the "For all changes" section of our PR template and let me know if you have any questions?
---
[GitHub] metron-bro-plugin-kafka issue #18: Remove version from show-plugin btest out...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
So, I did put together a PR meant to handle this @ metron-bro-plugin-kafka#8
Do you happen to know if your approach is more generally accepted? It feels somewhat strange to say "version" without providing a version number at the very least, but I could see the benefits of having a show-plugin btest that works pre- and post- 2.7.
---
[GitHub] metron-bro-plugin-kafka issue #18: Remove version from show-plugin btest out...
Posted by dopheide-esnet <gi...@git.apache.org>.
Github user dopheide-esnet commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
Please understand that I did that pull request for 20 different packages
precisely because it wasn't too time consuming. If it were a major change,
a new feature or something that I wanted, sure I'd jump through that hoop,
but for something so small I feel the additional form is a bit onerous. My
alternative was to fix the template in core and just send an email to the
Bro list to let everyone fix their packages on their own.
I'm happy to have a friendly debate about the potential impact of those
types of questionnaires on community contributions, but I'm not likely go
to back and fill it out.
-Dop
On Fri, Nov 16, 2018 at 7:42 AM JonZeolla <no...@github.com> wrote:
> Sounds good, I'm inclined to accept but there is a little bit of
> housekeeping to take care of first. Can you please take a look the "For all
> changes" section of our PR template and let me know if you have any
> questions?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/metron-bro-plugin-kafka/pull/18#issuecomment-439396209>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AKtpinCeI42OZ-YNgnD8UPTcqmrdth9Lks5uvsCngaJpZM4YeZl->
> .
>
---
[GitHub] metron-bro-plugin-kafka issue #18: Remove version from show-plugin btest out...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
Thanks for the taking the time you did to submit the PR @dopheide-esnet.
-1
Filling out the template to say "Yes reviewer, I tested, built, and check the lic etc " isn't a lot to ask and it helps the community, the reviewers a great deal. Having the courtesy to follow a project/communities policies when submitting is not a strange thing to expect.
You can close this.
Thanks again.
---
[GitHub] metron-bro-plugin-kafka pull request #18: Remove version from show-plugin bt...
Posted by dopheide-esnet <gi...@git.apache.org>.
Github user dopheide-esnet closed the pull request at:
https://github.com/apache/metron-bro-plugin-kafka/pull/18
---