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

Renamed some crates and changed their version to 0.1.0 #3683

Merged
merged 8 commits into from
Mar 16, 2023

Conversation

ptitSeb
Copy link
Contributor

@ptitSeb ptitSeb commented Mar 15, 2023

Renamed:

wasmer-wasi => wasmer-wasix 0.1.0
wasmer-wasi-types =>wasmer-wasix-types  0.1.0
wasmer-sys-utils 0.1.0
wasmer-vfs => virtfs 0.1.0
wasmer-vnet => virtnet 0.1.0
wasmer-wasi-local-networking  => virtnet-native  0.1.0
wasi-experimental-io-devices => wasix-experimental-io-devices 0.1.0

Copy link
Contributor

@theduke theduke left a comment

Choose a reason for hiding this comment

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

Did you not rename the directories on purpose?

The repo would become very inconsistent and confusing without aligning them with the crate names.

But this will be a very annoying change though for all open PRs, so if we do this we please need to merge #3653 first, because that's a large PR, and rebasing this PR on top of that one will be much easier than the other way around.

Alternative is to first merge without renaming the directories, try to get all large PRs in and then do the dir rename as a second PR.

@ptitSeb
Copy link
Contributor Author

ptitSeb commented Mar 15, 2023

Did you not rename the directories on purpose?

The repo would become very inconsistent and confusing without aligning them with the crate names.

But this will be a very annoying change though for all open PRs, so if we do this we please need to merge #3653 first, because that's a large PR, and rebasing this PR on top of that one will be much easier than the other way around.

Alternative is to first merge without renaming the directories, try to get all large PRs in and then do the dir rename as a second PR.

Yes, that was on purpose. I propose the crates that have ben rename to wasix instead of wasi have the folder renamed. I'm less sure about the other that already have a different folder name. But I can do it too.

How closed is the #3653 to be merged?

@theduke
Copy link
Contributor

theduke commented Mar 15, 2023

I think @john-sharratt is still fixing some stuff.

If we want to get this beta out quickly we can merge without renaming the directories and do that as a followup - but I do think we should get that PR in first either way because the conflicts would be annoying regardless.

I'm less sure about the other that already have a different folder name.

I actually see it almost the other way around - it's more important for virtfs / virtnet, etc, because those crates won't be changed back again. The ones that we want to eventually rename back to the old names could theoretically keep their old names. But I think it's better to rename them as well to keep the repo consistent.

@ptitSeb
Copy link
Contributor Author

ptitSeb commented Mar 15, 2023

I'm fine with renaming all the foler. It's just that it will be an anoying PR to handle for sure.

@syrusakbary
Copy link
Member

syrusakbary commented Mar 15, 2023

I've done a deeper review of the naming, and I don't think we should use virt-fs (virtfs is already a term in virtio).

I'd go with:

wasmer-wasi => wasmer-wasix 0.1.0
wasmer-wasi-types =>wasmer-wasix-types  0.1.0
wasmer-sys-utils 0.1.0
wasmer-vfs => virtual-fs 0.1.0
wasmer-vnet => virtual-net 0.1.0
wasmer-wasi-local-networking  => PUT THIS INSIDE OF virtual-net/host.rs (I don't see any reason why not?)
wasi-experimental-io-devices => wasix-experimental-io-devices 0.1.0

lib/cli/Cargo.toml Outdated Show resolved Hide resolved
lib/cli/Cargo.toml Outdated Show resolved Hide resolved
@syrusakbary syrusakbary merged commit 1fe1e51 into master Mar 16, 2023
@syrusakbary syrusakbary deleted the rename_crates branch March 16, 2023 17:23
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.

3 participants