Skip to content

Update adding maintainer section#2370

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
dmcgowan:update-maintainers-rules
Jun 8, 2018
Merged

Update adding maintainer section#2370
crosbymichael merged 1 commit intocontainerd:masterfrom
dmcgowan:update-maintainers-rules

Conversation

@dmcgowan
Copy link
Member

Updates the maintainers file to align with the process we have been following for adding maintainers and reviewers.

I do not believe this to be a rules change as it is just reflecting the rules we have already been following. Any maintainer is free to disagree though if this doesn't sound accurate.

@codecov-io
Copy link

codecov-io commented May 29, 2018

Codecov Report

Merging #2370 into master will increase coverage by 3.73%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2370      +/-   ##
==========================================
+ Coverage   41.26%   44.99%   +3.73%     
==========================================
  Files          66       92      +26     
  Lines        7850     9398    +1548     
==========================================
+ Hits         3239     4229     +990     
- Misses       4103     4486     +383     
- Partials      508      683     +175
Flag Coverage Δ
#linux 49.23% <ø> (?)
#windows 41.26% <ø> (ø) ⬆️
Impacted Files Coverage Δ
mount/temp.go 14.28% <0%> (ø)
sys/socket_unix.go 0% <0%> (ø)
sys/reaper.go 0% <0%> (ø)
sys/oom_unix.go 72.72% <0%> (ø)
content/local/store_unix.go 75% <0%> (ø)
sys/fds.go 0% <0%> (ø)
sys/epoll.go 0% <0%> (ø)
archive/time_unix.go 71.42% <0%> (ø)
archive/tar_unix.go 54.54% <0%> (ø)
sys/stat_unix.go 0% <0%> (ø)
... and 23 more

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 63522d9...7ba62b1. Read the comment docs.

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp
Copy link
Member

estesp commented May 29, 2018

Not going to merge until a plurality of maintainers have had a chance to look over it. However, I agree with @dmcgowan that this does not seem to be a rules change but more a clarification of how we have been practically applying the "add a maintainer" process.

MAINTAINERS Outdated
Copy link
Member

Choose a reason for hiding this comment

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

IIUC maintainership is organization-scoped, not repository-scoped?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think we should probably clarify that as well. This line only got touched for formatting. I would prefer to separate that change unless you want to add a commit to my branch to clarify 😄

@crosbymichael
Copy link
Member

LGTM

@estesp
Copy link
Member

estesp commented Jun 2, 2018

Any other comments from @containerd/containerd-maintainers ? Seems like we have a majority.

@hqhq
Copy link
Contributor

hqhq commented Jun 4, 2018

LGTM

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

/LGTM
just a nit

MAINTAINERS Outdated
Copy link
Member

Choose a reason for hiding this comment

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

nit.. their vote. Votes may take place on the mailing list or via pull request.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

MAINTAINERS Outdated
Copy link
Member

Choose a reason for hiding this comment

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

if you accept the above edit.. could /s/also// here.

Updates the maintainers file to align with the process
we have been following for adding maintainers and reviewers.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@dmcgowan dmcgowan force-pushed the update-maintainers-rules branch from dd0f31a to 7ba62b1 Compare June 7, 2018 23:06
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

/LGTM

@crosbymichael crosbymichael merged commit 35887db into containerd:master Jun 8, 2018
@dmcgowan dmcgowan deleted the update-maintainers-rules branch September 10, 2019 17:43
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.

8 participants