Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Using poll() in favor of select() in the XmlRPCDispatcher#1123

Closed
deng02 wants to merge 1 commit intoros:lunar-develfrom
deng02:xmlrpcdispatch-poll-redux
Closed

Using poll() in favor of select() in the XmlRPCDispatcher#1123
deng02 wants to merge 1 commit intoros:lunar-develfrom
deng02:xmlrpcdispatch-poll-redux

Conversation

@deng02
Copy link
Copy Markdown
Contributor

@deng02 deng02 commented Aug 8, 2017

This was originally committed as part of PR #833 but then rolled back after build failures in PR #981. Change has been reapplied to lunar-devel and tested on 64-bit xenial 16.04. I have been unable to reproduce the failures so resubmitting.

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Aug 8, 2017

I will look into the build failures.

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Aug 9, 2017

So the build failures appear to be the same set of failures occuring since build 250 (http://build.ros.org/job/Lpr__ros_comm__ubuntu_xenial_amd64/250/) In fact it looks like there hasn't been a green build since July 27 2017 build 227. I'm holding assuming failures are due to upstream issues still being resolved as per PR #1119.

@kmactavish
Copy link
Copy Markdown
Contributor

@ros-pull-request-builder retest this please

@kmactavish
Copy link
Copy Markdown
Contributor

I am very much in support of this PR. When this goes in, it can close my patch in #1113.

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Aug 11, 2017

@dirk-thomas We are really looking to get this change in. I was wondering if this PR is good as-is or if you would want anything added/removed in order for it to be merged for the next release of lunar?

@dirk-thomas
Copy link
Copy Markdown
Member

The last lunar release has been made 15 days ago. It hasn't been synced yet due to regressions. The priority will be to fix the regressions and getting a new release out which can be synced.

Currently this patch can't pass CI. Once CI is green it can be considered to be included in the release but I can't promise that at the moment.

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Aug 11, 2017

@dirk-thomas Thanks! I understand some build issues are being worked out but if there was anything that would help this PR outside of that, wanted to get a head start. As-is I will wait until CI is green and assume we'll revisit this when ready.

@dirk-thomas
Copy link
Copy Markdown
Member

The PR currently fails a similar test as the previous PR did. Therefore I don't think this is ready to be merged.

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Aug 14, 2017

Closing this PR as per #1133 which supercedes this.

@deng02 deng02 closed this Aug 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants