You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/08/29 10:19:17 UTC
[GitHub] [nifi-minifi-cpp] hunyadi-dev opened a new pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
hunyadi-dev opened a new pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884
This PR branched off from the WIP #851, checked for linter errors and then corrected two of the currently most prevalent ones (missing space before // and missing whitespace between code and comment).
Script used:
```bash
make linter |& egrep -v '^Done processing|^Ignoring' | grep "At least two spaces is best between code and comments" | tr ":" " " | cut -d" " -f1,2 | sort -rn -k1 -k2 | xargs -n 2 sh -c 'sed -i "" "$2s;//; //;" $1 && sed -i "" "$2s; //; //;" $1' sh
make linter |& egrep -v '^Done processing|^Ignoring' | grep "Should have a space between // and comment" | tr ":" " " | cut -d" " -f1,2 | sort -rn -k1 -k2 | xargs -n 2 sh -c 'sed -i "" "$2s;//;// ;" $1' sh
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484269857
##########
File path: nanofi/src/sitetosite/CRawSocketProtocol.c
##########
@@ -72,10 +72,10 @@ typedef struct {
int handShake(struct CRawSiteToSiteClient * client) {
if (client->_peer_state != ESTABLISHED) {
- //client->logger_->log_error("Site2Site peer state is not established while handshake");
+ // client->logger_->log_error("Site2Site peer state is not established while handshake");
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484260676
##########
File path: extensions/libarchive/FocusArchiveEntry.cpp
##########
@@ -130,7 +130,7 @@ void FocusArchiveEntry::onTrigger(core::ProcessContext *context, core::ProcessSe
}
archiveStack.push(archiveMetadata);
- //logger_->log_debug(archiveMetadata.toJsonString());
+ // logger_->log_debug(archiveMetadata.toJsonString());
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484259906
##########
File path: extensions/http-curl/tests/HTTPHandlers.h
##########
@@ -428,7 +428,7 @@ class HeartbeatHandler : public ServerAwareHandler {
void verify(struct mg_connection *conn) {
auto post_data = readPayload(conn);
- //std::cerr << post_data << std::endl;
+ // std::cerr << post_data << std::endl;
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484260410
##########
File path: extensions/jni/jvm/JniProcessContext.h
##########
@@ -69,8 +69,8 @@ DLL_EXPORT jstring JNICALL Java_org_apache_nifi_processor_JniProcessContext_getN
DLL_EXPORT jobject JNICALL Java_org_apache_nifi_processor_JniProcessContext_getControllerServiceLookup(JNIEnv *env, jobject obj);
-//getname
-//getControllerservicelookup
+// getname
+// getControllerservicelookup
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
lordgamez commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683277544
Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484268088
##########
File path: nanofi/include/sitetosite/CSiteToSite.h
##########
@@ -332,7 +332,7 @@ static int writeData(CTransaction * transaction, const uint8_t *value, int size)
}
static int readData(CTransaction * transaction, uint8_t *buf, int buflen) {
- //int ret = transaction->_stream->read(buf, buflen);
+ // int ret = transaction->_stream->read(buf, buflen);
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484259666
##########
File path: extensions/http-curl/tests/CivetStream.h
##########
@@ -119,7 +119,7 @@ class CivetStream : public io::BaseStream {
return readData(reinterpret_cast<uint8_t *>(&buf[0]), sizeof(t));
}
- //size_t pos;
+ // size_t pos;
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm closed pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
szaszm closed pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484263554
##########
File path: extensions/sftp/tests/FetchSFTPTests.cpp
##########
@@ -142,7 +142,7 @@ class FetchSFTPTestsFixture {
std::fstream file;
std::stringstream ss;
ss << src_dir << "/vfs/" << relative_path;
- utils::file::FileUtils::create_dir(utils::file::FileUtils::get_parent_path(ss.str())); // TODO
+ utils::file::FileUtils::create_dir(utils::file::FileUtils::get_parent_path(ss.str())); // TODO
Review comment:
I would refrain from deleting `TODO`-s, someone might know what they stand for. I could not trace it back what it could mean, it was added along with the function by Daniel Bakai.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484278402
##########
File path: nanofi/src/sitetosite/CRawSocketProtocol.c
##########
@@ -72,10 +72,10 @@ typedef struct {
int handShake(struct CRawSiteToSiteClient * client) {
if (client->_peer_state != ESTABLISHED) {
- //client->logger_->log_error("Site2Site peer state is not established while handshake");
+ // client->logger_->log_error("Site2Site peer state is not established while handshake");
return -1;
}
- //client->logger_->log_debug("Site2Site Protocol Perform hand shake with destination port %s", client->_port_id_str);
+ // client->logger_->log_debug("Site2Site Protocol Perform hand shake with destination port %s", client->_port_id_str);
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev edited a comment on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev edited a comment on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683658116
> > > Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
> >
> >
> > I am unaware of such plans.
>
> I would suggest integrating this linter check to github actions after they are fixed just to have them run automatically preventing further violations by any PRs. Or it can be done in a separate jira task if you prefer. I could open a new ticket for this if you would prefer to do it separately and I can open separate tickets for the pylint and shellcheck as well.
It is in fact integrated, we have a `linter` build system target (eg. `make linter`) which is built on at least one of the CI jobs. The directories to be checked were incorrectly specified, for this I have a WIP PR here: https://github.com/apache/nifi-minifi-cpp/pull/851
Once all of the fixes are merged, we can merge it and therefore improve our linter check coverage.
I can only do a few of these types of PR-s at the same time as fixing conflicts between them would most likely take up more time than doing the fixup.
Doing the changes themselves is easy as I already have scripts ready to fix them, however, there are a lot of changes to do (I would estimate it to take 30-40 PRs (!)):
```
cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | sort | wc -l
676
# Errors that occur at least 10 times:
cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | egrep -v "^\s*[1-9] " | wc -l
41
```
We are in no rush with this though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484278690
##########
File path: nanofi/src/sitetosite/CRawSocketProtocol.c
##########
@@ -153,7 +153,7 @@ int handShake(struct CRawSiteToSiteClient * client) {
if (client->_currentVersion >= 3) {
- //ret = client->_peer->writeUTF(client->_peer->getURL());
+ // ret = client->_peer->writeUTF(client->_peer->getURL());
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683658116
> > > Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
> >
> >
> > I am unaware of such plans.
>
> I would suggest integrating this linter check to github actions after they are fixed just to have them run automatically preventing further violations by any PRs. Or it can be done in a separate jira task if you prefer. I could open a new ticket for this if you would prefer to do it separately and I can open separate tickets for the pylint and shellcheck as well.
It is in fact integrated, we have a `linter` build system target which is built on at least one of the CI jobs. The directories to be checked were incorrectly specified, for this I have a WIP PR here: https://github.com/apache/nifi-minifi-cpp/pull/851
Once all of the fixes are merged, we can merge it and therefore improve our linter check coverage.
I can only do a few of these types of PR-s at the same time as fixing conflicts between them would most likely take up more time than doing the fixup.
Doing the changes themselves is easy as I already have scripts ready to fix them, however, there are a lot of changes to do (ETA: 40-50 PRs (!)):
```
cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | sort | wc -l
676
# Errors that occur at least 10 times:
cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | egrep -v "^\s*[1-9] " | wc -l
41
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
lordgamez commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683664280
> > > > Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
> > >
> > >
> > > I am unaware of such plans.
> >
> >
> > I would suggest integrating this linter check to github actions after they are fixed just to have them run automatically preventing further violations by any PRs. Or it can be done in a separate jira task if you prefer. I could open a new ticket for this if you would prefer to do it separately and I can open separate tickets for the pylint and shellcheck as well.
>
> It is in fact integrated, we have a `linter` build system target (eg. `make linter`) which is built on at least one of the CI jobs. The directories to be checked were incorrectly specified, for this I have a WIP PR here: #851
> Once all of the fixes are merged, we can merge it and therefore improve our linter check coverage.
>
> I can only do a few of these types of PR-s at the same time as fixing conflicts between them would most likely take up more time than doing the fixup.
>
> Doing the changes themselves is easy as I already have scripts ready to fix them, however, there are a lot of changes to do (I would estimate it to take 30-40 PRs (!)):
>
> ```
> cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | sort | wc -l
> 676
> # Errors that occur at least 10 times:
> cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | egrep -v "^\s*[1-9] " | wc -l
> 41
> ```
>
> We are in no rush with this though.
Thanks for the heads up! Now I see that the linter checks are part of the macos-xcode jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
fgerlits commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-684830203
I'm fine with it either way. I think this would be a good opportunity to remove these useless comments, but if you'd rather do that later in a separate pull request, that is OK, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484268701
##########
File path: nanofi/src/core/cstream.c
##########
@@ -149,7 +149,7 @@ int readUTFLen(uint32_t * utflen, cstream * stream) {
}
int readUTF(char * buf, uint32_t buflen, cstream * stream) {
- //return stream->impl->readData((uint8_t*)buf, buflen);
+ // return stream->impl->readData((uint8_t*)buf, buflen);
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683593188
> Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
I am unaware of such plans.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484266683
##########
File path: extensions/windows-event-log/TailEventLog.cpp
##########
@@ -115,7 +115,7 @@ void TailEventLog::onTrigger(const std::shared_ptr<core::ProcessContext> &contex
flowFile->addAttribute("event_time", getTimeStamp(event_record->TimeGenerated));
flowFile->addAttribute("event_type", typeToString(event_record->EventType));
- //flowFile->addAttribute("", event_message);
+ // flowFile->addAttribute("", event_message);
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484267747
##########
File path: nanofi/include/sitetosite/CSiteToSite.h
##########
@@ -267,7 +267,7 @@ typedef struct {
// Whether received data is available
int _dataAvailable;
- //org::apache::nifi::minifi::io::BaseStream* _stream;
+ // org::apache::nifi::minifi::io::BaseStream* _stream;
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
lordgamez commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683651016
> > Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
>
> I am unaware of such plans.
I would suggest integrating this linter check to github actions after they are fixed just to have them run automatically preventing further violations by any PRs. Or it can be done in a separate jira task if you prefer. I could open a new ticket for this if you would prefer to do it separately and I can open separate tickets for the pylint and shellcheck as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r484266987
##########
File path: main/MiNiFiWindowsService.cpp
##########
@@ -27,7 +27,7 @@
#include "MainHelper.h"
#include "core/FlowConfiguration.h"
-//#define DEBUG_SERVICE
+// #define DEBUG_SERVICE
Review comment:
Deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683792142
I agree that some of the comments are redundant, but I considered cleaning them up out-of-scope for this PR. I can add a commit cleaning them up though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#discussion_r480120751
##########
File path: extensions/http-curl/tests/HTTPHandlers.h
##########
@@ -428,7 +428,7 @@ class HeartbeatHandler : public ServerAwareHandler {
void verify(struct mg_connection *conn) {
auto post_data = readPayload(conn);
- //std::cerr << post_data << std::endl;
+ // std::cerr << post_data << std::endl;
Review comment:
should be deleted
##########
File path: extensions/http-curl/tests/CivetStream.h
##########
@@ -119,7 +119,7 @@ class CivetStream : public io::BaseStream {
return readData(reinterpret_cast<uint8_t *>(&buf[0]), sizeof(t));
}
- //size_t pos;
+ // size_t pos;
Review comment:
some of these should probably be deleted rather than fixing the whitespace in them; this is an example of that
##########
File path: main/MiNiFiWindowsService.cpp
##########
@@ -27,7 +27,7 @@
#include "MainHelper.h"
#include "core/FlowConfiguration.h"
-//#define DEBUG_SERVICE
+// #define DEBUG_SERVICE
Review comment:
should be deleted
##########
File path: nanofi/include/sitetosite/CSiteToSite.h
##########
@@ -267,7 +267,7 @@ typedef struct {
// Whether received data is available
int _dataAvailable;
- //org::apache::nifi::minifi::io::BaseStream* _stream;
+ // org::apache::nifi::minifi::io::BaseStream* _stream;
Review comment:
should be deleted
##########
File path: nanofi/src/sitetosite/CRawSocketProtocol.c
##########
@@ -72,10 +72,10 @@ typedef struct {
int handShake(struct CRawSiteToSiteClient * client) {
if (client->_peer_state != ESTABLISHED) {
- //client->logger_->log_error("Site2Site peer state is not established while handshake");
+ // client->logger_->log_error("Site2Site peer state is not established while handshake");
return -1;
}
- //client->logger_->log_debug("Site2Site Protocol Perform hand shake with destination port %s", client->_port_id_str);
+ // client->logger_->log_debug("Site2Site Protocol Perform hand shake with destination port %s", client->_port_id_str);
Review comment:
should be deleted
##########
File path: nanofi/src/sitetosite/CRawSocketProtocol.c
##########
@@ -153,7 +153,7 @@ int handShake(struct CRawSiteToSiteClient * client) {
if (client->_currentVersion >= 3) {
- //ret = client->_peer->writeUTF(client->_peer->getURL());
+ // ret = client->_peer->writeUTF(client->_peer->getURL());
Review comment:
should be deleted
##########
File path: extensions/jni/jvm/JniProcessContext.h
##########
@@ -69,8 +69,8 @@ DLL_EXPORT jstring JNICALL Java_org_apache_nifi_processor_JniProcessContext_getN
DLL_EXPORT jobject JNICALL Java_org_apache_nifi_processor_JniProcessContext_getControllerServiceLookup(JNIEnv *env, jobject obj);
-//getname
-//getControllerservicelookup
+// getname
+// getControllerservicelookup
Review comment:
should be deleted
##########
File path: extensions/libarchive/FocusArchiveEntry.cpp
##########
@@ -130,7 +130,7 @@ void FocusArchiveEntry::onTrigger(core::ProcessContext *context, core::ProcessSe
}
archiveStack.push(archiveMetadata);
- //logger_->log_debug(archiveMetadata.toJsonString());
+ // logger_->log_debug(archiveMetadata.toJsonString());
Review comment:
should be deleted
##########
File path: nanofi/src/api/nanofi.cpp
##########
@@ -544,7 +544,7 @@ processor *add_python_processor(flow *flow, processor_logic* logic) {
return nullptr;
}
auto lambda = [logic](core::ProcessSession *ps, core::ProcessContext *cx) {
- logic(static_cast<processor_session*>(ps), static_cast<processor_context*>(cx)); //Meh, sorry for this
+ logic(static_cast<processor_session*>(ps), static_cast<processor_context*>(cx)); // Meh, sorry for this
Review comment:
Does http://whatthecommit.com take user submissions? This comment would be a good candidate for that. :)
##########
File path: nanofi/include/sitetosite/CSiteToSite.h
##########
@@ -332,7 +332,7 @@ static int writeData(CTransaction * transaction, const uint8_t *value, int size)
}
static int readData(CTransaction * transaction, uint8_t *buf, int buflen) {
- //int ret = transaction->_stream->read(buf, buflen);
+ // int ret = transaction->_stream->read(buf, buflen);
Review comment:
should be deleted
##########
File path: extensions/sftp/tests/FetchSFTPTests.cpp
##########
@@ -142,7 +142,7 @@ class FetchSFTPTestsFixture {
std::fstream file;
std::stringstream ss;
ss << src_dir << "/vfs/" << relative_path;
- utils::file::FileUtils::create_dir(utils::file::FileUtils::get_parent_path(ss.str())); // TODO
+ utils::file::FileUtils::create_dir(utils::file::FileUtils::get_parent_path(ss.str())); // TODO
Review comment:
the comment should be deleted
##########
File path: nanofi/src/core/cstream.c
##########
@@ -149,7 +149,7 @@ int readUTFLen(uint32_t * utflen, cstream * stream) {
}
int readUTF(char * buf, uint32_t buflen, cstream * stream) {
- //return stream->impl->readData((uint8_t*)buf, buflen);
+ // return stream->impl->readData((uint8_t*)buf, buflen);
Review comment:
should be deleted
##########
File path: nanofi/src/sitetosite/CRawSocketProtocol.c
##########
@@ -72,10 +72,10 @@ typedef struct {
int handShake(struct CRawSiteToSiteClient * client) {
if (client->_peer_state != ESTABLISHED) {
- //client->logger_->log_error("Site2Site peer state is not established while handshake");
+ // client->logger_->log_error("Site2Site peer state is not established while handshake");
Review comment:
should be deleted
##########
File path: extensions/windows-event-log/TailEventLog.cpp
##########
@@ -115,7 +115,7 @@ void TailEventLog::onTrigger(const std::shared_ptr<core::ProcessContext> &contex
flowFile->addAttribute("event_time", getTimeStamp(event_record->TimeGenerated));
flowFile->addAttribute("event_type", typeToString(event_record->EventType));
- //flowFile->addAttribute("", event_message);
+ // flowFile->addAttribute("", event_message);
Review comment:
should be deleted
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev edited a comment on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev edited a comment on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-683658116
> > > Is there a plan/jira ticket for also running these checks (with possibly an additionally shellcheck for shell files and flake8/pylint check for python files) in the CI as github actions as well?
> >
> >
> > I am unaware of such plans.
>
> I would suggest integrating this linter check to github actions after they are fixed just to have them run automatically preventing further violations by any PRs. Or it can be done in a separate jira task if you prefer. I could open a new ticket for this if you would prefer to do it separately and I can open separate tickets for the pylint and shellcheck as well.
It is in fact integrated, we have a `linter` build system target (eg. `make linter`) which is built on at least one of the CI jobs. The directories to be checked were incorrectly specified, for this I have a WIP PR here: https://github.com/apache/nifi-minifi-cpp/pull/851
Once all of the fixes are merged, we can merge it and therefore improve our linter check coverage.
I can only do a few of these types of PR-s at the same time as fixing conflicts between them would most likely take up more time than doing the fixup.
Doing the changes themselves is easy as I already have scripts ready to fix them, however, there are a lot of changes to do (I would estimate it to take 40-50 PRs (!)):
```
cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | sort | wc -l
676
# Errors that occur at least 10 times:
cat linter_errors.txt | cut -d":" -f3- | sort | uniq -c | egrep -v "^\s*[1-9] " | wc -l
41
```
We are in no rush with this though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on pull request #884: MINIFICPP-1203 - Correct linter reported missing spaces around comments
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on pull request #884:
URL: https://github.com/apache/nifi-minifi-cpp/pull/884#issuecomment-684832461
I will remove the obsolete/redundant comments. Will do that later though, among the things I am working on, this has the least priority.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org