Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

go.mod: github.com/lima-vm/vz/v4 #5

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Oct 18, 2024

Rename github.com/Code-Hex/vz/v3 to github.com/lima-vm/vz/v4 so that our fork can be consumed without the replace directive in go.mod.

Fix #3

Also remove the contents specific to https://github.com/Code-Hex/vz (CONTRIBUTING.md, FUNDING.yml, etc.)

Copy link

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

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

I would also change

#define RAISE_REASON_MESSAGE                                                                               \
    "This may possibly be a bug due to library handling errors.\n"                                         \
    "I would appreciate it if you could report it to https://github.com/Code-Hex/vz/issues/new/choose\n\n" \
    "Information: %@\n"

in virtualization_helper.h

Apart from this and the comment about the unrelated changes, the PR looks good to me.

1. If this is your first time, please read our contributor guidelines: https://github.com/Code-Hex/vz/blob/master/CONTRIBUTING.md
2. Please create a new issue before creating this PR. However, You can continue it without creating issues if this PR fixes any documentations such as typo.
3. Do not send Pull Requests for large (150 ~ lines) code changes. If so, I am not motivated to review your code. Basically, I write the code.
<!-- Thanks for sending a pull request!
Copy link

@cfergeau cfergeau Oct 18, 2024

Choose a reason for hiding this comment

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

This change and the removal of FUNDING.yml and CONTRIBUTING.md are not strictly related to the module renaming.

Copy link
Member Author

Choose a reason for hiding this comment

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

Those files were specific to https://github.com/Code-Hex/vz and not appropriate for https://github.com/lima-vm/vz

Choose a reason for hiding this comment

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

Yeah I understand, and I agree they should be removed/rewritten. For clarity, I would have put these changes in a separate commit with the explanation you just added.

@AkihiroSuda
Copy link
Member Author

virtualization_helper.h

done

Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

I think the part about the Wiki could be a bit clearer that the quoted text is from the upstream author (to make sure who "I" is in "Therefore, I have compiled the knowledge I have gathered so far into this wiki"). But I guess it is good enough.

Rename `github.com/Code-Hex/vz/v3` to `github.com/lima-vm/vz/v4`
so that our fork can be consumed without the `replace` directive in `go.mod`.

Fix issue 3

Also remove the contents specific to https://github.com/Code-Hex/vz
(`CONTRIBUTING.md`, `FUNDING.yml`, etc.)

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
Copy link
Member Author

I think the part about the Wiki could be a bit clearer that the quoted text is from the upstream author (to make sure who "I" is in "Therefore, I have compiled the knowledge I have gathered so far into this wiki").

done

Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@AkihiroSuda AkihiroSuda merged commit ac2dbb8 into lima-vm:main Oct 18, 2024
8 of 9 checks passed
@AkihiroSuda AkihiroSuda added this to the v4.0.0 milestone Oct 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider changing the package name to github.com/lima-vm/vz/v4
3 participants