-
Notifications
You must be signed in to change notification settings - Fork 16
run_parsers: locally tag latest vehicle versions by reading version.h to include master metadata #13
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
base: main
Are you sure you want to change the base?
Conversation
9a7f157 to
c0ae965
Compare
ES-Alexander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be generally working - nice!
Here are some suggestions to clean up the code and behaviour a bit :-)
| version_path = Path(f'{self.repository_path}/{vehicle_type}/version.h') | ||
| version_path2 = Path(f'{self.repository_path}/Ardu{vehicle_type}/version.h') | ||
| path = version_path if version_path.exists() else version_path2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love that this is duplicated in get_version_for_tag.
scripts/run_parsers.py
Outdated
| if vehicle_type == 'Sub': | ||
| self.repository.create_tag(f'Ardu{vehicle_type}-{major}.{minor}.{patch}') | ||
| else: | ||
| self.repository.create_tag(f'{vehicle_type}-{major}.{minor}.{patch}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to just create the base tag, and add 'Sub': 'Sub' to the valid_name_map (instead of having a special "Ardu" case for it).
That simplifies the tagging code, and future-proofs the parsing for if/when ArduSub ends up getting included in the combined ArduPilot releases.
|
I am curious whether this will break things when the release gets tagged properly, given the existing mechanism to not re-generate existing files... |
patrickelectric
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File "/mnt/king/git/blue/ParameterRepository/./scripts/run_parsers.py", line 55
major, minor, _ = self.get_version_from_version_h(file)
^
IndentationError: unindent does not match any outer indentation level
|
Rebase over master |
Co-authored-by: ES-Alexander <[email protected]>
90d36d0 to
8683ae4
Compare
ES-Alexander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noice :-)
| 'ArduCopter': 'Copter', | ||
| 'ArduPlane': 'Plane', | ||
| 'ArduSub': 'Sub', | ||
| 'Sub': 'Sub', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the (alphabetical) order of the others, I guess this should be at the bottom, after Rover?
|
@patrickelectric I'm unsure how the groundskeeper is intended to behave when an ArduPilot tag is newer than the last generation from the parsers, but there have been code changes in this repo that are more recent than the tag. At the moment it seems to just skip the new tags because of that (but that's not the fault of this PR). We can potentially just wait a few hours for the next commit in ArduPilot to happen to confirm that side of things is working properly. It does already seem to be generating the tags as expected though, and running without the groundskeeper seems to also generate the files as expected 👍 |
|
I ended up doing some final fixes on this and including it in #16, partly so that the new Blimp inclusion could actually have a generation (since it has no stable tags yet). That PR will close this one when it gets merged. |
This tags master with the vehicle's version.h version, so that it can be consumed in the following stages, generating a metadata folder for master with the current version number