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

[Citadel] Document deprecation of playback <path> SDF param #424

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

mabelzhang
Copy link
Contributor

@mabelzhang mabelzhang commented Oct 21, 2020

Documentation to address #293 for Citadel.

Discussed with @chapulina and we decided to do the following:
Blueprint & Citadel:

  • Keep log_playback.sdf since the distros have already been released and the file installed.
  • Update tutorial to say <path> is not recommended.
  • Update header comment in example file. I thought it made more sense to update log_playback.sdf rather than log_record_*.sdf, because it's really the playback file that's being replaced, and they just have to use --playback [arg]. Nothing changes for recording.
  • Add a note in Migration.md

Essentially deprecate <path> for Blueprint and Citadel, remove it in Dome.

Will make followup PR for Blueprint.
Dome will be in a separate PR with different items - log_playback.sdf has already been removed in Dome, documentation needs to be updated, and some test code needs to be changed, so it'll take a bit longer.

@mabelzhang mabelzhang added the 🏰 citadel Ignition Citadel label Oct 21, 2020
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks!

@chapulina chapulina merged commit 1680ebb into ign-gazebo3 Oct 21, 2020
@chapulina chapulina deleted the deprecate_playback_path_citadel branch October 21, 2020 20:41
doisyg pushed a commit to wyca-robotics/ign-gazebo that referenced this pull request Dec 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants