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

Fix suit -> suite #71

Merged
merged 2 commits into from
Oct 2, 2017
Merged

Fix suit -> suite #71

merged 2 commits into from
Oct 2, 2017

Conversation

drdanz
Copy link
Member

@drdanz drdanz commented Sep 28, 2017

Warning: breaking change

I don't think it is worth supporting both the suit and suite syntax in the xml parsing and in the arguments, it would require a lot of effort just to support a typo. I suggest we just break compatibility now instead.

in master we can ensure that the RTF version is ok by changing:

-find_package(RTF 1.2)
+find_package(RTF 1.2 EXACT)

in devel we can change it to

-find_package(RTF 1.2)
+find_package(RTF 1.3)

@traversaro
Copy link
Member

Fix #47 .

@traversaro
Copy link
Member

traversaro commented Sep 28, 2017

I hate to always be the "backwards compatibility" guy, but the amount of tests that we have in the vvv-school repository and the students' forked repositories in my opinion call for a proper tick-tock cycle of deprecation/removal. I can take care of that, hopefully before this week.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 56.299% when pulling 1fbd3fe on drdanz:fix/suit into 2a920ff on robotology:devel.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 56.299% when pulling 1fbd3fe on drdanz:fix/suit into 2a920ff on robotology:devel.

@pattacini
Copy link
Member

I would go for the smoothest change possible.

@pattacini
Copy link
Member

Please trigger me once merged so that I'll take care of updating vvv-school.

@traversaro
Copy link
Member

Support and tests for the legacy options added in 3dd861e .

@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 59.055% when pulling 3dd861e on drdanz:fix/suit into 2a920ff on robotology:devel.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 59.055% when pulling 3dd861e on drdanz:fix/suit into 2a920ff on robotology:devel.

@drdanz
Copy link
Member Author

drdanz commented Oct 2, 2017

Thanks @traversaro for supporting the legacy option... I think we can merge this now since it's no longer breaking icub-tests and vvv-school... anyone against merging?

@aerydna aerydna merged commit 0107761 into robotology:devel Oct 2, 2017
@aerydna
Copy link
Contributor

aerydna commented Oct 2, 2017

merged, thanks!

@drdanz drdanz deleted the fix/suit branch October 2, 2017 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants