search

container-storage-interface / spec

Container Storage Interface (CSI) Specification.
Apache License 2.0
1.34k stars 373 forks source link

Corrects a mistake in the specs #342

Closed msolefonte closed 5 years ago

msolefonte commented 5 years ago

This patch modifies the zone value from R2 to Z2 to respect the format and the explanation of the example. It is not an major change so I think it is not logical to sign it.

If you want to close the PR and just modify the code by yourselves, there would be no problem.

jdef commented 5 years ago

Thanks for the PR. Please run make to re-generate the protobuf file and dependent golang libs.

jdef commented 5 years ago

It looks like a bunch of build/scratch files were added to this PR that should not have been.

msolefonte commented 5 years ago

@jdef That is the result of running make. Anyway, I do not know why should I do it if I only modified a Markdown file from the GitHub website.

Please feel free to notify me about how to realize that change without affecting to the main project. I want to help but I am not used to Go.

msolefonte commented 5 years ago

Any update? I still wonder what are the important files to be pushed. I think that spec.md is obviously a must but why the make? I am really curious about this.

jdef commented 5 years ago

make does a few things:

make clean all is normally enough to rebuild everything. It looks like you've committed a bunch of files that were not previously committed to the repo. I don't know how you got into this state, but we cannot accept the PR as-is.

msolefonte commented 5 years ago

Thanks for the explanation. I deleted the last commit and ran make clean all. This time only csi.proto, lib/go/csi/csi.pb.go and, of course, spec.md have been modified. I hope it is finally fine.

jdef commented 5 years ago

Thanks. It now looks like you need to rebase against master and re-run make clean all to resolve the conflicts.

msolefonte commented 5 years ago

I hope this is enough. Excuse me but I am not used to the environment.

saad-ali commented 5 years ago

Thanks @msolefonte. Can you please make sure to sign the CLA - see https://github.com/container-storage-interface/spec/blob/master/CONTRIBUTING.md#licence-and-cla

msolefonte commented 5 years ago

Hello @saad-ali. Are you going to release a new version closely? I have some time problems right now and if I were able to sing and upload the CLA this weekend I would be really grateful. I have to fully read it first, thought.

Also, I have a problem because I have two GitHub accounts (personal and professional) and, because of git misconfiguration the commits are bound to the wrong account. To avoid problems with my company (and to sing CLA as an individual), I am going to reject commits and commit again (running make again), so I need a bit of time for it too.

If the release is planned for this week, please feel free to read my commits and reproduce them.

msolefonte commented 5 years ago

@jdef @saad-ali This PR has been continued in #353, so this one can be closed