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

MAINTAINERS: Add Yan Song as a reviewer #108

Merged
merged 1 commit into from
Jul 27, 2022

Conversation

changweige
Copy link
Member

@changweige changweige commented Jul 20, 2022

Yan Song @imeoer is a core developer and maintainer of Nydus container image service, especially on the nydusd runtime/filesystem and building nydus container image via nydus-image, nydusify and Harbor Accelleratioin Service sub-project. For nydus-snapshotter he had completed features like the setting up nydus snapshots using Erofs/Fscache fs driver, which directly works on top of Linux in-kernel filesystem as container rootfs with lazyload capability, an essential feature in the upcoming nydus release. In addition, he contributed to the feature that nydus-snapshotter can directly use stargz container image to provide container rootfs with many other neat fixups.

He is eager to help community users with rich experience.

I'd like to invite him as a nydus-snapshotter reviewer if he would approve :)

Needs explicit LGTM from @imeoer and 1/3 of the nydus-snapshotter Committers:

@changweige changweige requested review from eryugey and a team July 20, 2022 03:36
@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022

Codecov Report

Merging #108 (159a3a4) into main (55ec5f8) will decrease coverage by 1.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
- Coverage   28.77%   27.57%   -1.20%     
==========================================
  Files          17       17              
  Lines        1536     1494      -42     
==========================================
- Hits          442      412      -30     
+ Misses       1030     1024       -6     
+ Partials       64       58       -6     
Impacted Files Coverage Δ
pkg/filesystem/fs/blob_manager.go 44.44% <0.00%> (-11.12%) ⬇️
pkg/filesystem/fs/fs.go 1.16% <0.00%> (-5.63%) ⬇️
cmd/containerd-nydus-grpc/pkg/command/flags.go 76.56% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55ec5f8...159a3a4. Read the comment docs.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

left a comment about the formatting of the file (otherwise lgtm 😅)

MAINTAINERS Outdated Show resolved Hide resolved
MAINTAINERS Outdated Show resolved Hide resolved
Signed-off-by: Changwei Ge <gechangwei@bytedance.com>
@eryugey
Copy link
Collaborator

eryugey commented Jul 22, 2022

Or we could add Yan Song as committer as well?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@changweige
Copy link
Member Author

Or we could add Yan Song as a committer as well?

I personally feel OK about adding Yan Some as a committer based on his remarkable contribution. But I am not sure if it is normal to directly add a committer according to Containerd's governance.
As we have already got two votes from @containerd/reviewers, I suggest adding Yan Song as a reviewer at this stage and discussing it later on.

@changweige
Copy link
Member Author

Seven days have passed. Let's merge this and welcome @imeoer to join the nydus-snapshotter maintainers team :)

@changweige changweige merged commit e1e2baa into containerd:main Jul 27, 2022
@changweige changweige deleted the add-maintainer branch August 24, 2022 01:19
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.

6 participants