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.

Sign up for GitHub

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

Jump to bottom

test: test initramfs #8

Merged
merged 1 commit into from
Nov 5, 2023
Merged

test: test initramfs #8

merged 1 commit into from
Nov 5, 2023

Conversation

WoodenMaiden
Copy link
Contributor

No description provided.

@WoodenMaiden WoodenMaiden changed the title feat(test): test initramfs Draft: feat(test): test initramfs Oct 24, 2023
@WoodenMaiden WoodenMaiden changed the title Draft: feat(test): test initramfs feat(test): test initramfs Oct 24, 2023
@WoodenMaiden WoodenMaiden marked this pull request as draft October 24, 2023 07:15
@WoodenMaiden WoodenMaiden marked this pull request as ready for review October 24, 2023 07:23
@lucido-simon
Copy link
Contributor

Hi! Thanks for the PR. Can you squash the two commits together please ?

@WoodenMaiden WoodenMaiden marked this pull request as draft October 24, 2023 14:51
@WoodenMaiden WoodenMaiden marked this pull request as ready for review October 25, 2023 08:36
alexis-langlet
alexis-langlet requested changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@alexis-langlet alexis-langlet left a comment

Choose a reason for hiding this comment

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

Nice work
The only real issue of all this is that you don't use mocks as mocks but as different implementations

initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
alexis-langlet
alexis-langlet reviewed Oct 30, 2023
View reviewed changes
initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
alexis-langlet
alexis-langlet previously approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@alexis-langlet alexis-langlet left a comment

Choose a reason for hiding this comment

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

Really nice job, thank you

@WoodenMaiden WoodenMaiden changed the title feat(test): test initramfs test: test initramfs Oct 30, 2023
lucido-simon
lucido-simon requested changes Nov 4, 2023
View reviewed changes
Copy link
Contributor

@lucido-simon lucido-simon left a comment

Choose a reason for hiding this comment

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

LGTM, except a few formatting nits. gj !

initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
initramfs/src/image.rs Outdated Show resolved Hide resolved
@WoodenMaiden @lucido-simon
feat(test): image and export
8c6bb7c 
Signed-off-by: WoodenMaiden <yann.pomie@laposte.net>
@lucido-simon
Copy link
Contributor

I've taken the liberty of making a small change: renaming blobSum of the LayerMetadata struct to blob_sum. This allows removing the #[allow(non_snake_case)], by replacing it by #[serde(rename_all = "camelCase")] in image.rs.

Other than that, PR LGTM.

@lucido-simon lucido-simon merged commit 62eddb9 into main Nov 5, 2023
4 checks passed
@lucido-simon lucido-simon deleted the test-initramfs branch November 5, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucido-simon lucido-simon lucido-simon approved these changes

@alexis-langlet alexis-langlet alexis-langlet left review comments

Assignees

@WoodenMaiden WoodenMaiden

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@WoodenMaiden @lucido-simon @alexis-langlet