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

Move the implementation of BDA::wipe into the static header module #1584

Closed
mulkieran opened this issue Jul 15, 2019 · 2 comments · Fixed by #1661
Closed

Move the implementation of BDA::wipe into the static header module #1584

mulkieran opened this issue Jul 15, 2019 · 2 comments · Fixed by #1661
Assignees
Labels
sub-issue issue that has a parent

Comments

@mulkieran
Copy link
Member

mulkieran commented Jul 15, 2019

I think that BDA::wipe is invoked from outside the metadata module, so the method itself should be left. A new method should be created inside the static_header module.

Here's the new proposal:

  1. Hoist the device_identifiers method right out of the BDA implementation and into the module top-level. This can be done by defining it, as a public method, in metadata/mod.rs. A lot of callers which currently call BDA::device_identifiers will need to be updated accordingly. Internal tests which call device_identifiers should be updated to call StaticHeader::setup instead.

  2. Simultaneously:

  • Add another method in metadata/mod.rs called disown_device. Have this method call the StaticHeader::wipe method directly.

  • Move BDA::wipe into the StaticHeader implementation.

  • Update calls to BDA::wipe so that they instead call disown_device wherever necessary (or StaticHeader::wipe if they are metadata internal testing code).

Note that, after this, all StaticHeader-specific methods should have been moved into the StaticHeader implementation. So in the next step it should be possible to move the entire implementation into a separate module, one of the main parts of the goal of #1573.

@GuillaumeGomez
Copy link
Contributor

Will do it in the next days then!

@mulkieran
Copy link
Member Author

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sub-issue issue that has a parent
Projects
None yet
2 participants