You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by "spnettec (via GitHub)" <gi...@apache.org> on 2023/02/17 16:56:29 UTC
[PR] Various Changes for #1 (plc4x)
spnettec opened a new pull request, #812:
URL: https://github.com/apache/plc4x/pull/812
fix OutOfMemoryError
fix S7 driver
add s7-200
add Timeout-HandlingVarious ChangesExtended syntax of tags to add "encoding" to s7 string fields
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "spnettec (via GitHub)" <gi...@apache.org>.
spnettec closed pull request #812: Various Changes for #1
URL: https://github.com/apache/plc4x/pull/812
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "spnettec (via GitHub)" <gi...@apache.org>.
spnettec commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1439764820
I will close this PR
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1438772466
I intentionally didn't pull the generated code changes into that other PR I created as it was swamping the usefull bits of the PR
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1439629013
Well the thing is: I'm currently working most of my time, cleaning up things. So I will definitely not approve an approach like this. If the others do, they will have to do the house-keeping for it. I know how stuff like this happened in the past ... stuff was merged with the promise to clean up later, but then nothing happened and I'm currently working on cleaning it up.
This is not the way Open-Source projects (at Apache work) ... we don't like huge code-drops like this. Even if the amount of code isn't that huge, but the number of changes are.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "ottlukas (via GitHub)" <gi...@apache.org>.
ottlukas commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1439678584
> I saw the discuss. I know the problem. But i think we should resolve user urgent problem. We can change that at next version. For me the additional encoding for string tags is important now.
There is a balance between you specific urgent problem and a stable plc4x framework / open source in a foundation like the ASF. If the refactoring of code is extensive and can not forseen it is against the basic principle of Community over Code. If you can convince the community on the mailing list to accept your PR then it would work. To be realistic.... the changes are too big and it would be hard to get a majority with this amount of changes. So we like to pull in you really nice changes in a slower pace and decouple it by looking first and the non-breaking elements and for the string topic you already saw the dicussion on the mailing list. I hope that helps you to understand a little better as we are aiming to have a community around the code and a stable framework where developers are comfortable on supporting and evolving PLC4X.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "spnettec (via GitHub)" <gi...@apache.org>.
spnettec commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1438804202
I saw the discuss. I know the problem. But i think we should resolve user urgent problem. We can change that at next version. For me the additional encoding for string tags is important now.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1438760435
can you run a `maven clean install` and commit that? looks like there is a bunch of formatting which affected a huge amount of files.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "spnettec (via GitHub)" <gi...@apache.org>.
spnettec commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1434922124
This is based fix. I need to ensure that these basic fixes can be merged before doing further merges
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1435340276
It's also a great example why it's not that good of an idea to work on a fork and not have the changes pulled back ... I hope I'll have the time to pull some of the changes back this weekend, but I was currently working on getting some EIP driver changes in. Currently a bit much for one person to do ... would be cool, if some of the other comitters could also jump in and have a look at https://github.com/apache/plc4x/pull/784 first ... that was the one where I wen't though most changes.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1435341133
But ... thanks a lot for officially creating a PR ... this IS really highly appreciated. Now all we need to do is get you back and then, if we work a lot closer from then on, this will be a lot better for all :-)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "spnettec (via GitHub)" <gi...@apache.org>.
spnettec commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1438784327
It is not huge PR. I run maven clean install to make sure everything compiles fine. You can ignore generated code changes
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "spnettec (via GitHub)" <gi...@apache.org>.
spnettec commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1434925081
By the way, I'm heyoulin. I have two github account
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1435337419
This is a huge PR to chew on ... and I did have a look when I created the big PR for you ... we'll definitely not take all changes and we'll do some things differently. So I would not like to simply merge all of this. Also can't I review 2500 files. I think I wen't through most of them in the PR that I created, but I'm not going to do it again.
I think we need to manually pull individual changes into individual PRs and over time merge in the stuff we want to see merged.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1438776199
just to clear up that comment was directed at @spnettec
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Various Changes for #1 (plc4x)
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1438795814
Yeah ... but we won't merge this PR ... as it contains many changes that we would have to discuss first and currently would probably solve differently (Like the one with the additional encoding for string tags) .... we're currently disucssing a similar extension on the dev-list, wehre discussions like this belong for an apache project.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org