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

Add CONTRIBUTING.md #400

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Conversation

jsturtevant
Copy link
Contributor

Adding a contributing doc which includes guidance for adding and removing new shims.

Copy link
Collaborator

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

Thanks @jsturtevant , this is very needed!

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated

We welcome new shims, though you can also host them in your own repositories as well and use the `coantainerd-shim-wasm` crate. We recognize that the project is moving fast to having them in this repository can reduce the need for changes as well for discoverability.

Please open an issue before submitting a PR for discussion to make sure it is a good fit. As a general rule, we want shims to be adopting WASI spec. Since we are not experts in every runtime we also need a commitment from the runtime owners to contribute to maintenance of the shim.
Copy link
Member

Choose a reason for hiding this comment

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

This is related to #338

A good way to make sure the new shim is compilant is to run a suit of WASI tests that we provide (not right now, but may in the future)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool I will mention we are looking at this and may become a hard requirement in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, please check wording

CONTRIBUTING.md Outdated

## Removing Shims

If a shim in not maintained for 6 months it will be subject to removal. Before removal:
Copy link
Member

Choose a reason for hiding this comment

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

Is ambiguous to me if this is saying the "shim" itself is not maintained, or the "runtime" underneath the shim is not maintainer for 6 months.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how to define this. Open to wording and we can discuss more in the meeting next week.

I don't think it is just the runtime its self (but that is a good thing to call out here). But also want to avoid a situation if there is a bug in the runtime that surfaces in our test suite or need changes to runtime to support a feature but no one is around to help resolve quickly even if the runtime is actively maintained.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've taken another shot at this

Copy link
Collaborator

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

I think we still need to discuss the Adding new shims and Removing shims sections with stakeholders, but the rest is very valuable technical content.
Can we move those two sections to a follow up PR to unblock the rest of this PR?

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@jsturtevant
Copy link
Contributor Author

I think we still need to discuss the Adding new shims and Removing shims sections with stakeholders, but the rest is very valuable technical content. Can we move those two sections to a follow up PR to unblock the rest of this PR?

yes, makes sense. I will split them up

Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant
Copy link
Contributor Author

jsturtevant commented Nov 27, 2023

Moved the adding/removing details to #408

@Mossaka
Copy link
Member

Mossaka commented Nov 28, 2023

Could you please take another look before I merge this in? @jprendes

@Mossaka
Copy link
Member

Mossaka commented Nov 30, 2023

Going to merge this in.

@Mossaka Mossaka merged commit df454fd into containerd:main Nov 30, 2023
43 checks passed
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