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

loop: Add test to setup GPT and ESP on raw disk image #93

Merged
merged 1 commit into from Jun 15, 2022
Merged

loop: Add test to setup GPT and ESP on raw disk image #93

merged 1 commit into from Jun 15, 2022

Conversation

michael-schaller
Copy link
Contributor

@michael-schaller michael-schaller commented Jun 9, 2022

Add test for setting up a raw disk image with a GPT and ESP.
This is a regression test for commit b9684a71fca7
("block, loop: support partitions without scanning").

Signed-off-by: Michael Schaller misch@google.com

Copy link
Collaborator

@kawasaki kawasaki left a comment

Choose a reason for hiding this comment

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

Hi @michael-schaller
Thank you for this PR. The code logic looks good. Let me make a few comments on other things.

  • Apache license is noted in the file header comment. However, GPL3+ is the license for blktests (some files have GPL2+). To keep license of blktests simple, I wish to see GPL3+ for this new test case also.
  • The commit title is rather lengthy. The additional note in parenthesis (regression ...) can be removed, I think.
  • The line of commit message is too long. I wish to see 72 characters each in the commit message lines.

Hope these requests are reasonable for you.

@michael-schaller michael-schaller changed the title loop: Add test to setup GPT and ESP on raw disk image (regression test for commit b9684a71fca7) loop: Add test to setup GPT and ESP on raw disk image Jun 13, 2022
@michael-schaller
Copy link
Contributor Author

Done. Please take another look.

@kawasaki
Copy link
Collaborator

@michael-schaller Thanks for the license change! However, the 2nd commit for license change does not have your Signed-off-by tag, which all blktests commits have. Could you add the tag?

Also, single commit is the better record than the two commits. Could you squash the two commits into one and force push to your git branch? Or if it takes too much time for you, please let me know. I can do that squash work.

Add test for setting up a raw disk image with a GPT and ESP.
This is a regression test for commit b9684a71fca7
("block, loop: support partitions without scanning").

Signed-off-by: Michael Schaller <misch@google.com>
@michael-schaller
Copy link
Contributor Author

Done. Please take another look.

@kawasaki kawasaki merged commit af97b55 into osandov:master Jun 15, 2022
@kawasaki
Copy link
Collaborator

@michael-schaller Thanks for the updates. Applied to the master branch.

@michael-schaller
Copy link
Contributor Author

Thanks. 😃

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

2 participants