Skip to content

Updating prefix formats #260

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

Merged
merged 2 commits into from
Nov 6, 2018

Conversation

verult
Copy link
Contributor

@verult verult commented Aug 3, 2018

This PR makes explicit the implicit restriction that underscores "_" are not allowed in the prefix format, as described in the wiki link. It also proposes to further restrict prefixes to lower-case letters only. Internally in k8s, topology is represented by labels, which unfortunately doesn't allow either underscores or capitalized letters.

This should be a fairly low-risk change, but definitely still worth broadcasting in the next spec release.

@verult
Copy link
Contributor Author

verult commented Aug 5, 2018

@jieyu @jdef @saad-ali

@jdef
Copy link
Member

jdef commented Sep 5, 2018

I think I'm fine with this, though it is a "breaking" change since it's more restrictive than what was there before.

Is this something that we should target for the v0.4 milestone?

@jdef jdef requested review from jdef, jieyu and saad-ali and removed request for jdef and jieyu September 5, 2018 12:02
@verult
Copy link
Contributor Author

verult commented Sep 19, 2018

Is there currently a planned date for the v0.4 cut?

@jdef
Copy link
Member

jdef commented Oct 4, 2018

@saad-ali also suggested a change, removing the "reverse" from "reverse domain name notation"

@jdef jdef added this to the v1.0 milestone Oct 4, 2018
@jdef jdef added the breaking-change Breaks backwards compat label Oct 4, 2018
@verult
Copy link
Contributor Author

verult commented Oct 25, 2018

Will add that to the PR

…wed; Changed reverse domain name notation to forward domain name notation.
@verult
Copy link
Contributor Author

verult commented Oct 26, 2018

Updated. In the recommended domain name format there is no underscore. Do we want to keep it?

@verult
Copy link
Contributor Author

verult commented Oct 26, 2018

Any guidance on how to fix the Travis failure? The protoc versions match between Travis and my local machine (3.5.1) and protobuf versions for protoc-gen-go also match (1.2.0)

@verult verult changed the title Updating topology prefix format Updating prefix formats Oct 29, 2018
@saad-ali
Copy link
Member

saad-ali commented Nov 2, 2018

LGTM

@saad-ali
Copy link
Member

saad-ali commented Nov 2, 2018

@verult This looks like #226

Try this:

make clobber
make clean
make all

@verult
Copy link
Contributor Author

verult commented Nov 2, 2018

Updated, PTAL!

@saad-ali
Copy link
Member

saad-ali commented Nov 2, 2018

Closes #280

Copy link
Member

@saad-ali saad-ali left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jdef jdef left a comment

Choose a reason for hiding this comment

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

LGTM

@saad-ali saad-ali merged commit f624381 into container-storage-interface:master Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Breaks backwards compat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants