[GitHub] [plc4x] hutcheb commented on a change in pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb commented on a change in pull request #225:
URL: https://github.com/apache/plc4x/pull/225#discussion_r561792144
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xNamespace.java
##
@@ -1,11 +1,20 @@
/*
- * Copyright (c) 2019 the Eclipse Milo Authors
Review comment:
The EPL headers for the Plc4xNamespace and OPCUAServer files were added
in as I wasn't sure how much of the code reflected the Milo example.
I am comfortable that there is no code that needs to be licensed under EPL
remaining.
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
##
@@ -115,9 +71,27 @@
Map monitoredList = new HashMap<>();
public Plc4xCommunication () {
+
+}
+
+@Override
+protected void onStartup() {
driverManager = new PooledPlcDriverManager();
}
+@Override
Review comment:
It overrides an abstract method, I don't really have much to do on
shutdown. I've added a comment to indicate that.
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:
us...@infra.apache.org
[GitHub] [plc4x] hutcheb merged pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb merged pull request #225: URL: https://github.com/apache/plc4x/pull/225 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: us...@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on a change in pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
ottobackwards commented on a change in pull request #225:
URL: https://github.com/apache/plc4x/pull/225#discussion_r561843725
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
##
@@ -65,6 +65,7 @@
private final Logger logger = LoggerFactory.getLogger(getClass());
private final Integer DEFAULT_TIMEOUT = 100;
private final Integer DEFAULT_RETRY_BACKOFF = 5000;
+private final DataValue BAD_RESPONSE = new DataValue(new Variant(null),
StatusCode.BAD);
Review comment:
should this be static?
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
##
@@ -115,9 +71,27 @@
Map monitoredList = new HashMap<>();
public Plc4xCommunication () {
+
+}
+
+@Override
+protected void onStartup() {
driverManager = new PooledPlcDriverManager();
}
+@Override
Review comment:
ah, IC, you can't tell if it is abstract base or interface, always
catches me. thanks
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:
us...@infra.apache.org
[GitHub] [plc4x] hutcheb commented on pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb commented on pull request #225: URL: https://github.com/apache/plc4x/pull/225#issuecomment-764564895 Otto, all good ideas, I have updated the code. 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: us...@infra.apache.org
Re: [DISCUSS] Change the naming scheme for release-tags?
+1.
Also will call a discussion on the IoTDB mail-list to keep consistent :D
Best,
---
Xiangdong Huang
School of Software, Tsinghua University
Otto Fowler 于2021年1月21日周四 下午8:29写道：
> Sounds fine to me.
> +1
>
> > On Jan 21, 2021, at 05:26, Christofer Dutz
> wrote:
> >
> > Hi all,
> >
> > a little discussion on the IoTDB list made me realize that I never
> actually discussed this here and I'd like to do it before we start the next
> release.
> >
> > This will be the first containing Go content. Go resolves dependencies
> quite differently as it doesn't ship binaries, but you instead reference
> Git repositories (GitHub is actually best integrated type). You reference a
> version of a Go library via git tags. So if you want a version 1.2.3, you
> actually reference a tag named "v1.2.3". I have seen that if you don't
> stick to that some tools and utils might have problems.
> >
> > So I would like to ask you, if you would agree that we change the
> release-tag naming scheme from "releases/{full-version}" to
> "v{full-version}" ... it doesn't have any side-effects to any other tools.
> And it would eliminate the little discrepancy in the naming "release"
> branch, but "releases/" as prefix for tags (because if there's a branch
> called "release" we can't prefix a tag with "release/".
> >
> > So what do you think?
> >
> > Chris
> >
>
>
[GitHub] [plc4x] ottobackwards commented on a change in pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
ottobackwards commented on a change in pull request #225:
URL: https://github.com/apache/plc4x/pull/225#discussion_r561843725
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
##
@@ -65,6 +65,7 @@
private final Logger logger = LoggerFactory.getLogger(getClass());
private final Integer DEFAULT_TIMEOUT = 100;
private final Integer DEFAULT_RETRY_BACKOFF = 5000;
+private final DataValue BAD_RESPONSE = new DataValue(new Variant(null),
StatusCode.BAD);
Review comment:
should this be static?
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
##
@@ -115,9 +71,27 @@
Map monitoredList = new HashMap<>();
public Plc4xCommunication () {
+
+}
+
+@Override
+protected void onStartup() {
driverManager = new PooledPlcDriverManager();
}
+@Override
Review comment:
ah, IC, you can't tell if it is abstract base or interface, always
catches me. thanks
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:
us...@infra.apache.org
[GitHub] [plc4x] hutcheb merged pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb merged pull request #225: URL: https://github.com/apache/plc4x/pull/225 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: us...@infra.apache.org
Re: [DISCUSS] Change the naming scheme for release-tags?
Sounds fine to me.
+1
> On Jan 21, 2021, at 05:26, Christofer Dutz wrote:
>
> Hi all,
>
> a little discussion on the IoTDB list made me realize that I never actually
> discussed this here and I'd like to do it before we start the next release.
>
> This will be the first containing Go content. Go resolves dependencies quite
> differently as it doesn't ship binaries, but you instead reference Git
> repositories (GitHub is actually best integrated type). You reference a
> version of a Go library via git tags. So if you want a version 1.2.3, you
> actually reference a tag named "v1.2.3". I have seen that if you don't stick
> to that some tools and utils might have problems.
>
> So I would like to ask you, if you would agree that we change the release-tag
> naming scheme from "releases/{full-version}" to "v{full-version}" ... it
> doesn't have any side-effects to any other tools. And it would eliminate the
> little discrepancy in the naming "release" branch, but "releases/" as prefix
> for tags (because if there's a branch called "release" we can't prefix a tag
> with "release/".
>
> So what do you think?
>
> Chris
>
[GitHub] [plc4x] hutcheb commented on pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb commented on pull request #225: URL: https://github.com/apache/plc4x/pull/225#issuecomment-764564895 Otto, all good ideas, I have updated the code. 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: us...@infra.apache.org
[GitHub] [plc4x] hutcheb commented on a change in pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb commented on a change in pull request #225:
URL: https://github.com/apache/plc4x/pull/225#discussion_r561792739
##
File path:
plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
##
@@ -115,9 +71,27 @@
Map monitoredList = new HashMap<>();
public Plc4xCommunication () {
+
+}
+
+@Override
+protected void onStartup() {
driverManager = new PooledPlcDriverManager();
}
+@Override
Review comment:
It overrides an abstract method, I don't really have much to do on
shutdown. I've added a comment to indicate that.
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:
us...@infra.apache.org
[GitHub] [plc4x] hutcheb commented on a change in pull request #225: Add encryption handler for OPC UA - Minor fix for Kafka Connector
hutcheb commented on a change in pull request #225: URL: https://github.com/apache/plc4x/pull/225#discussion_r561792144 ## File path: plc4j/integrations/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xNamespace.java ## @@ -1,11 +1,20 @@ /* - * Copyright (c) 2019 the Eclipse Milo Authors Review comment: The EPL headers for the Plc4xNamespace and OPCUAServer files were added in as I wasn't sure how much of the code reflected the Milo example. I am comfortable that there is no code that needs to be licensed under EPL remaining. 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: us...@infra.apache.org
[DISCUSS] Change the naming scheme for release-tags?
Hi all,
a little discussion on the IoTDB list made me realize that I never actually
discussed this here and I'd like to do it before we start the next release.
This will be the first containing Go content. Go resolves dependencies quite
differently as it doesn't ship binaries, but you instead reference Git
repositories (GitHub is actually best integrated type). You reference a version
of a Go library via git tags. So if you want a version 1.2.3, you actually
reference a tag named "v1.2.3". I have seen that if you don't stick to that
some tools and utils might have problems.
So I would like to ask you, if you would agree that we change the release-tag
naming scheme from "releases/{full-version}" to "v{full-version}" ... it
doesn't have any side-effects to any other tools. And it would eliminate the
little discrepancy in the naming "release" branch, but "releases/" as prefix
for tags (because if there's a branch called "release" we can't prefix a tag
with "release/".
So what do you think?
Chris
