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.

Sign up for GitHub

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

Jump to bottom

SIG-storage charter #2461

Merged
merged 2 commits into from
Aug 23, 2018
Merged

SIG-storage charter #2461

merged 2 commits into from
Aug 23, 2018

Conversation

saad-ali
Copy link
Member

@saad-ali saad-ali commented Aug 3, 2018 •
edited
Loading

Charter for @kubernetes/sig-storage-misc

/assign @childsb for initial pass. Will assign to TOC for approval once we have internal SIG storage consensus.

Also closes #2467

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/storage Categorizes an issue or PR as relevant to SIG Storage. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 3, 2018
@childsb
Copy link
Contributor

childsb commented Aug 3, 2018

/approve

j-griffith
j-griffith reviewed Aug 3, 2018
View reviewed changes
sig-storage/charter.md Outdated
* Storage Classes and Dynamic Provisioning
* Kubernetes volume plugins
* Container Storage Interface (CSI)
* Secret, ConfigMap, DownwardAPI Volumes
Copy link

Choose a reason for hiding this comment

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

Don't know if this should be more explicit that its Secrets, ConfigMap etc related to Volumes? I know that's pretty implicit due to the doc and such but just a nit.

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to mention enptydir volumes too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

j-griffith
j-griffith reviewed Aug 3, 2018
View reviewed changes
Copy link

@j-griffith j-griffith left a comment

Choose a reason for hiding this comment

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

Minor nit but overall looks like a solid charter to me.

@saad-ali saad-ali force-pushed the sig-storage-charter branch from b9ae477 to eb6db48 Compare August 3, 2018 19:39
j-griffith
j-griffith approved these changes Aug 3, 2018
View reviewed changes
Copy link

@j-griffith j-griffith left a comment

Choose a reason for hiding this comment

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

LGTM

msau42
msau42 reviewed Aug 3, 2018
View reviewed changes
sig-storage/charter.md Outdated
* Storage Classes and Dynamic Provisioning
* Kubernetes volume plugins
* Container Storage Interface (CSI)
* Secret Volumes, ConfigMap Volumes, DownwardAPI Volumes, EmptyDirVolumes (co-owned with SIG-Node)
Copy link
Member

Choose a reason for hiding this comment

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

nit: space between EmptyDir and Volumes

@bgrant0607
Copy link
Member

/committee steering

@k8s-ci-robot k8s-ci-robot added the committee/steering Denotes an issue or PR intended to be handled by the steering committee. label Aug 6, 2018
@saad-ali saad-ali force-pushed the sig-storage-charter branch from 8480db7 to 95118a3 Compare August 8, 2018 20:32
@jdumars jdumars mentioned this pull request Aug 9, 2018
Closed
@spiffxp
Copy link
Member

spiffxp commented Aug 10, 2018

Can you split the sigs.yaml change out of this? @brendandburns is listed as the primary reviewer for this and is out next week. I'd like to get the sigs.yaml piece merged sooner.

@saad-ali saad-ali force-pushed the sig-storage-charter branch from 95118a3 to 30a3935 Compare August 11, 2018 01:00
@saad-ali
Copy link
Member Author

@spiffxp Done: #2519

@cblecker
Copy link
Member

/uncc

@k8s-ci-robot k8s-ci-robot removed the request for review from cblecker August 12, 2018 00:50
@philips
Copy link
Contributor

philips commented Aug 15, 2018

/lgtm

cc @quinton-hoole can you take a look since @brendanburns is OOO

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2018
@spiffxp
Copy link
Member

spiffxp commented Aug 16, 2018

/cc @michelleN
expressed an interest in reviewing as well

@k8s-ci-robot k8s-ci-robot requested a review from michelleN August 16, 2018 01:48
spzala
spzala reviewed Aug 18, 2018
View reviewed changes
sig-storage/charter.md Outdated
SIG Storage delegates subproject approval to Technical Leads. See [Subproject creation - Option 1].

[sig-governance]: https://github.com/kubernetes/community/blob/master/committee-steering/governance/sig-governance.md
[sigs.yaml]: https://github.com/kubernetes/community/blob/master/sigs.yaml#L1454
Copy link
Member

Choose a reason for hiding this comment

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

sigs.yaml seems not used and not needed here. (Also per the changes in charter template use of sigs.yaml is discouraged and recommended to be replaced with SIG's README.)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is copy paste from https://github.com/kubernetes/community/blob/master/committee-steering/governance/sig-governance.md#project-management as recommended by SIG Charter template.

Copy link
Member

Choose a reason for hiding this comment

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

@saad-ali thanks, not sure if I am missing something here and if so please ignore my comment but what I noticed is,

  1. The charter.md seems not using sigs.yaml anywhere, so I don't think related link is needed
  2. Even if we are using it, then referencing it by line number is not good as numbers will change, the charter template is already modified, e0d60fd

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops I misread this comment. I thought it was about Subproject creation for some reason. Removed the stray sigs.yaml link

@saad-ali saad-ali force-pushed the sig-storage-charter branch from 30a3935 to 33058d1 Compare August 23, 2018 22:22
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 23, 2018
@spzala
Copy link
Member

spzala commented Aug 23, 2018 •
edited
Loading

@saad-ali :-) np, looks good to me now!

@ghost
Copy link

ghost commented Aug 23, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot assigned ghost Aug 23, 2018
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 23, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: childsb, quinton-hoole

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2018
@k8s-ci-robot k8s-ci-robot merged commit e600529 into kubernetes:master Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@spzala spzala spzala left review comments

@j-griffith j-griffith j-griffith approved these changes

@childsb childsb Awaiting requested review from childsb

@michelleN michelleN Awaiting requested review from michelleN

Assignees

@philips philips

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. committee/steering Denotes an issue or PR intended to be handled by the steering committee. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kubernetes-csi/* repos need to be added to sigs.yaml and have OWNERS files added
10 participants
@saad-ali @childsb @bgrant0607 @spiffxp @cblecker @philips @spzala @k8s-ci-robot @j-griffith @msau42