Skip to content

MINIFICPP-910 - Extend StringUtils with string join capability#585

Closed
arpadboda wants to merge 1 commit intoapache:masterfrom
arpadboda:MINIFICPP-910
Closed

MINIFICPP-910 - Extend StringUtils with string join capability#585
arpadboda wants to merge 1 commit intoapache:masterfrom
arpadboda:MINIFICPP-910

Conversation

@arpadboda
Copy link
Contributor

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 MINIFICPP-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?
  • 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.

@@ -0,0 +1,162 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content of this file is mostly moved from the header, the changes I made were to make linter happy. Some of these functions still leave space for improvement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quite nice.

Copy link
Contributor

@phrocker phrocker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I summarized my thoughts better in the ticket ( from my phone ) while watching a movie, so I'll let that stand.

@arpadboda arpadboda force-pushed the MINIFICPP-910 branch 2 times, most recently from bdb1331 to 495bb52 Compare June 11, 2019 12:54
Copy link
Contributor

@phrocker phrocker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @arpadboda I'll launch this on some devices and merge if good.

As a project we generally don't commit dead production code -- but I can understand the desire to close this one out so I'll merge if it passes those tests. Thanks!

I think it's totally fair to leverage a need for code like this on others' pull requests as it begins a chain of testability.

btw, I don't suspect the builds to fail with this, but I perform these builds on anything in libminifi -- so I'll merge it when that is successfully complete, hoping in the next few hours. Thanks again!

@arpadboda
Copy link
Contributor Author

@phrocker : thanks, usage will come soon! :)

@phrocker
Copy link
Contributor

phrocker commented Jun 24, 2019

@arpadboda I hope to get this merged tomorrow. One of the builds passed, but the other devices are working on higher priority PRs.

raspberry pi 3 actually passes now on all variants of the OS so I think that one is good. Thanks! I'll check the other devices once those other PRs are finished building.

Copy link
Contributor

@phrocker phrocker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just have one more platform to succeed. if it does I'll merge this. Thanks!

@asfgit asfgit closed this in ca2a68d Jun 24, 2019
nghiaxlee pushed a commit to nghiaxlee/nifi-minifi-cpp that referenced this pull request Jul 8, 2019
This closes apache#585.

Signed-off-by: Marc Parisi <phrocker@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants