You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by phrocker <gi...@git.apache.org> on 2017/04/04 01:32:18 UTC
[GitHub] nifi-minifi-cpp pull request #73: MINIFI-254: Incremental update for linter ...
GitHub user phrocker opened a pull request:
https://github.com/apache/nifi-minifi-cpp/pull/73
MINIFI-254: Incremental update for linter changes
This particular commit reduces the number < 200.
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFI-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)?
- [ ] Is your initial contribution a single, squashed commit?
### For code 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)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-254
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi-cpp/pull/73.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 #73
----
commit 2717886b316af480912e4a50192899e570c28356
Author: Marc Parisi <ph...@apache.org>
Date: 2017-04-03T20:27:24Z
MINIFI-254: Incremental update for linter changes
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi-cpp issue #73: MINIFI-254: Incremental update for linter changes
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/73
@phrocker code and builds looked good as presented.
unfortunately the rebase after #71 is not a clean one. I was looking through the conflicts, and they are not overly bad, but was wondering if you would be able to take a look at them? They are substantiative enough that I think I would want to post a branch for review if I made the changes.
Let me know what works from your side. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi-cpp issue #73: MINIFI-254: Incremental update for linter changes
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/73
@apiri squashed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi-cpp issue #73: MINIFI-254: Incremental update for linter changes
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/73
@apiri I agree with your change. I think in hindsight your approach is much smarter from that perspective.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi-cpp pull request #73: MINIFI-254: Incremental update for linter ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi-cpp/pull/73
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi-cpp issue #73: MINIFI-254: Incremental update for linter changes
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/73
This is pretty fantastic and all the changes look good. Built and ran a simple flow.
Would you mind if I switched around the order of linter and testing in the travis file?
From:
`mkdir ./build && cd ./build && cmake .. && make && make linter && ./tests`
to
`mkdir ./build && cd ./build && cmake .. && make && ./tests && make linter`
The primary motivation is that in the review process being able to catch the test defects is nice and then if there are minor linting issues, it may or may not require follow up. In the current scenario, a single linter issue will prevent tests from executing.
Regardless, thanks for cleaning all this up and letting us automate the process using CI.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi-cpp issue #73: MINIFI-254: Incremental update for linter changes
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/73
Verified builds in OS X and Linux **after** rebasing. \U0001f625
Looks good and verified linter picking up issues for formatting changes.
Will get this merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---