Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eloquent patch release 1 #115

Merged
merged 8 commits into from Jan 21, 2020
Merged

Conversation

@mjcarroll
Copy link
Member

DCO is unhappy, at a minimum.

@ivanpauno
Copy link
Member Author

ivanpauno commented Jan 17, 2020

DCO is unhappy, at a minimum.

The commit where the checker is failing is signed. It seems to be a false-positive, but I can add my signature if that makes the checker happier.

@mjcarroll mjcarroll added this to In Progress in Eloquent Patch Release 1 Jan 17, 2020
@ivanpauno
Copy link
Member Author

DCO is unhappy, at a minimum.

The commit where the checker is failing is signed. It seems to be a false-positive, but I can add my signature if that makes the checker happier.

Sorry, there is actually one commit failing because of a missing DCO, and some others failing because of strange reasons. I can add my signature in all of them to avoid the checker to fail, but I don't know if that's correct. Also, if the commit is already in master, IDK if I should add an extra signature.

@dirk-thomas
Copy link
Member

If you co-author a commit (e.g. a cherry pick or merge) you need to sign that commit too to pass DCO.

BMarchi and others added 8 commits January 17, 2020 18:13
Signed-off-by: Brian Ezequiel Marchi <brian.marchi65@gmail.com>
Signed-off-by: ivan <ivanpauno@gmail.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: ivan <ivanpauno@gmail.com>
Signed-off-by: Brian Ezequiel Marchi <brian.marchi65@gmail.com>
Signed-off-by: ivan <ivanpauno@gmail.com>
* Use imperative mood in constructor docstrings.

Fixes D401 in pycodestyle 5.0.0 and flake8.

Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>

* Use imperative mood in docstring.

Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>

* Use imperative mood in docstrings.

Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
Signed-off-by: ivan <ivanpauno@gmail.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: ivan <ivanpauno@gmail.com>
… during shutdown (#104)

* check for shutdown while waiting for a service response to avoid hang during shutdown

Signed-off-by: William Woodall <william@osrfoundation.org>

* fix typo in logger call

Signed-off-by: William Woodall <william@osrfoundation.org>
Signed-off-by: ivan <ivanpauno@gmail.com>
* Add frontend remap test

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Pass data_type parameter to remap entity

This resolves an issue where frontend remaps are not parsed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: ivan <ivanpauno@gmail.com>
@ivanpauno ivanpauno force-pushed the ivanpauno/eloquent-patch-release-1 branch from 32b13f7 to cd285ab Compare January 17, 2020 21:15
@ivanpauno
Copy link
Member Author

If you co-author a commit (e.g. a cherry pick or merge) you need to sign that commit too to pass DCO.

That wasn't exactly the failure that the DCO checker was showing, but done.

@mjcarroll
Copy link
Member

Can you fix that linter error?

@ivanpauno
Copy link
Member Author

ivanpauno commented Jan 20, 2020

Can you fix that linter error?

Are you talking about the errors here ros2/launch#370 (comment)?

Does are from launch repository, and not from launch_ros.
They are there just because I didn't backport ros2/launch#362.
We usually don't backport those commits, but I can add it to the list if you want.

@ivanpauno
Copy link
Member Author

CI is the same as in: ros2/launch#370 (comment)

@wjwwood
Copy link
Member

wjwwood commented Jan 21, 2020

I'm concerned about how #106 breaks behavior and API, but I think that probably no one is using it (or that probably no one would be affected by it) and it would make more sense to fix it. But we've been wrong about that in the past.

So, I'd tentatively vote to backport it and then make an announcement with the patch release letting people know. But I'll leave that up to @mjcarroll to decide how to proceed.

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from my concerns with #106 being backported, lgtm.

@mjcarroll
Copy link
Member

I'll add it to the announcement. If it causes issues, we can revert, but I also believe it to be unlikely.

@ivanpauno
Copy link
Member Author

ivanpauno commented Jan 21, 2020

@wjwwood I agree with your concerns. I proposed it just because the bug is to ugly, and there's no way of fixing it without breaking API.

@ivanpauno ivanpauno merged commit f586dbb into eloquent Jan 21, 2020
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/eloquent-patch-release-1 branch January 21, 2020 20:01
@ivanpauno ivanpauno moved this from In Progress to Needs Release in Eloquent Patch Release 1 Jan 21, 2020
@mjcarroll mjcarroll moved this from Needs Release to Released in Eloquent Patch Release 1 Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants