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

Fixed version firmware type #8250

Merged
merged 3 commits into from
Nov 12, 2017
Merged

Fixed version firmware type #8250

merged 3 commits into from
Nov 12, 2017

Conversation

garfieldG
Copy link
Contributor

Fixed the version tag to number and version tag to vendor version number to return dev version in case of any local modifications or in case there's a dash before firmware type in tags that support vendor version.

Fixed the version tag to number and version tag to vendor version number to return dev version in case of any local modifications or in case there's a dash before firmware type in tags that support vendor  version.
@dagar
Copy link
Member

dagar commented Nov 7, 2017

Thanks! I've been meaning to look at this.

@dagar
Copy link
Member

dagar commented Nov 7, 2017

The circleci failure is real (trivial style change), jenkins sitl01 is broken globally.

@dagar dagar added the bug label Nov 7, 2017
@dagar dagar added this to the Release v1.7.0 milestone Nov 7, 2017
@dagar dagar requested review from dagar and darioxz November 7, 2017 15:53
@garfieldG
Copy link
Contributor Author

@dagar Thanks for response! Passed the circleci check

@mrpollo
Copy link
Contributor

mrpollo commented Nov 8, 2017

@dagar good to merge?

Copy link
Contributor

@darioxz darioxz left a comment

Choose a reason for hiding this comment

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

@garfieldG, thanks for the improvement. Could you please add new test cases to the corresponding unit test?

Please have a look at test_versioning.cpp.

Local modification cases and a dash before firmware type in tags that support vendor version cases.
@garfieldG
Copy link
Contributor Author

@darioxz Added new test cases as requested.

Copy link
Contributor

@darioxz darioxz left a comment

Choose a reason for hiding this comment

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

@garfieldG thanks, seems good! 👍

@dagar
Copy link
Member

dagar commented Nov 9, 2017

Looks good. I'm going to do some quick testing and get this in for v1.7.0.

Thanks!

@dagar dagar merged commit cddea6f into PX4:master Nov 12, 2017
@garfieldG garfieldG deleted the version_tag_fix branch November 12, 2017 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants