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

Review README files #356

Closed
rojkov opened this issue Apr 2, 2020 · 10 comments · Fixed by #460
Closed

Review README files #356

rojkov opened this issue Apr 2, 2020 · 10 comments · Fixed by #460
Assignees
Labels
docs Documentation related issue

Comments

@rojkov
Copy link
Contributor

rojkov commented Apr 2, 2020

Some files got outdated. For example cmd/gpu_plugin/README.md describes the installation procedure like we still use dep instead of go mod.

@mythi
Copy link
Contributor

mythi commented Apr 2, 2020

We just did a bunch of updates. What is outdated?

like we still use dep instead of go mod.

Where's this?

@rojkov
Copy link
Contributor Author

rojkov commented Apr 2, 2020

Particularly this
https://github.com/intel/intel-device-plugins-for-kubernetes/blob/master/cmd/gpu_plugin/README.md#getting-the-source-code

It's not needed nowadays to clone the repo into a specific path.

@rojkov
Copy link
Contributor Author

rojkov commented Apr 2, 2020

This section https://github.com/intel/intel-device-plugins-for-kubernetes#running-e2e-tests needs to be included in the README's ToC.

@mythi
Copy link
Contributor

mythi commented Apr 2, 2020

It's not needed nowadays to clone the repo into a specific path.

true. I touched this part last time (f145541) and IIRC my thinking was that this gives a consistent starting point/path for each higher level section without any bigger re-work...

@msivosuo msivosuo assigned bart0sh and unassigned rojkov Jun 16, 2020
@mythi
Copy link
Contributor

mythi commented Jun 16, 2020

Next steps:

  • in all of the READMEs, highlight the container image availability and the possibility to deploy using kubectl apply without cloning the repo
  • change git cloning (go get does not work) to be path agnostic (GOPATH no longer relevant)

@pohly
Copy link

pohly commented Jun 16, 2020

the possibility to deploy using kubectl apply without cloning the repo

Also make sure that those deployment files always pull an image that is compatible with the deployment.

In PMEM-CSI, we ensure that for releases, but not for the development branch, and already had issue reports twice because users were using deployment files from an old revision of our master branch with the latest, incompatible "canary" images.

@tkhred
Copy link

tkhred commented Jun 17, 2020

Hi, based on the following PR, the plugin has already supported k8s 1.18.

#365

I think it is a critical problem. (Actually, we need several days to know that the plugin supports 1.18 or not.)

Could you consider to update the top page README first?

@mythi
Copy link
Contributor

mythi commented Jun 17, 2020

Could you consider to update the top page README first?

I just opened a PR to work on updates in the top page README. What is the concern? In principle, the release branches should be used for work that requires stability.

@tkhred
Copy link

tkhred commented Jun 17, 2020

Sorry that I asked without checking your project plan. Our team will test the master branch with k8s 1.18 and I believe we can wait for the official v0.18 release.

@mythi
Copy link
Contributor

mythi commented Jun 17, 2020

@tkhred no problem at all. we're planning to make a v0.18 shortly (#251) you'r reminder is a valid task for #251!

@msivosuo msivosuo assigned rojkov and unassigned bart0sh Sep 2, 2020
@msivosuo msivosuo added the docs Documentation related issue label Sep 15, 2020
rojkov added a commit to rojkov/intel-device-plugins-for-kubernetes that referenced this issue Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants