You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2021/07/17 04:58:45 UTC
[GitHub] [zookeeper] bigmarvin opened a new pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
bigmarvin opened a new pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727
Following up https://github.com/apache/zookeeper/pull/1722, I'm applying the change on master instead of 3.5.x as advised.
As an alternative to https://github.com/apache/zookeeper/pull/1726, this change employs bundle plugin to build another artifact of classifier "osgi".
The advantage would be bundle plugin remains employed to maintain the topology of versioned packages, and the original artifact without classifier is left untouched.
The disadvantage would be we've one more artifact delivered in this project.
```
$ ls zookeeper-server/target/ | grep jar$
zookeeper-3.5.9.jar
zookeeper-3.5.9-javadoc.jar
zookeeper-3.5.9-osgi.jar
zookeeper-3.5.9-sources.jar
zookeeper-3.5.9-tests.jar
```
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] asfgit closed pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] nkalmar commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
nkalmar commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-891700219
Merged to master, 3.7, 3.6 and 3.5 branch. Thank you @bigmarvin
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] bigmarvin commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
bigmarvin commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-886351323
May I ask if anything else I can help with for this PR to merge? I'm not very familiar with the process in this repo while it's been a while after last update, so please feel free to let me know.
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] bigmarvin closed pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
bigmarvin closed pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] bigmarvin commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
bigmarvin commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-891809771
Thank you @eolivelli and @nkalmar! Github should have provided more emoji XD
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-891334664
We need a second reviewer ('committer' in ASF terms) to sponsor this patch.
Tagging @nkalmar @anmolnar @maoling @ztzg @breed @hanm
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] bigmarvin commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
bigmarvin commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-891026269
_Renewing the PR once more..._
May I ask if anything else I can help with for this PR to merge? I'm not very familiar with the process in this repo while it's been a while after last update, so please feel free to let me know.
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] bigmarvin edited a comment on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
bigmarvin edited a comment on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-891026269
_Hoping everything is well and renewing the PR again_
May I ask if anything else I can help with for this PR to merge? I'm not very familiar with the process in this repo while it's been a while after last update, so please feel free to let me know.
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] bigmarvin commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
bigmarvin commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-881315840
> this is great stuff!
>
> is it possible to add some kind of integration tests (an additional Maven module I guess) that try to load the library as an OSGI bundle.
>
> this way we will be sure that we are not breaking it in the future.
After few days' struggling, I'm afraid I won't add integration tests for this change. There is some compliance testing provided by OSGi official, so the problem would be how to wrap that BND pieces into some integration case.
http://felix.apache.org/documentation/development/using-the-osgi-compliance-tests.html
I didn't really find some programmatically appropriate way to make it happen, except for forking another process to run CT in the case. Given this new artifact is packed by some new employed plugin independently, I believe it won't disappear magically if nobody touches that plugin. If somebody does, on the other hand, I don't think a case can stop him/her.
Does this explanation sound good enough for you, @eolivelli? It's some good point that testing should be part of every change, while I wonder if we can take the risk for this particular one. 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.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1727: ZOOKEEPER-4331: add headers back in osgi artifact
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1727:
URL: https://github.com/apache/zookeeper/pull/1727#issuecomment-891334859
I am fine with committing this patch in this form
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org