Skip to content

Conversation

@TrumanLing
Copy link

it will be used in 'image-tools' or other project for checking regular
expression of 'refs', which writes as "Object names in the refs
subdirectories MUST NOT include characters outside of the set of "A" to
"Z", "a" to "z", "0" to "9", the hyphen -, the dot ., and the underscore
_." at https://github.com/opencontainers/image-spec/blob/master/image-layout.md

Signed-off-by: Ling FaKe [email protected]

var (
// refRegexp for string ref should be complied with description "Object names in the refs subdirectories MUST NOT include characters outside of the set of "A" to "Z", "a" to "z", "0" to "9", the hyphen -, the dot ., and the underscore _." at https://github.com/opencontainers/image-spec/blob/master/image-layout.md.
RefsRegexp = regexp.MustCompile(`^[a-zA-Z0-9\-\.\_]+?$`)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shift to specs-go/v1/layout.go, which already has some image-layout stuff (and may get more soon, #414)?

Copy link
Author

Choose a reason for hiding this comment

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

Got it!

@TrumanLing
Copy link
Author

@wking it is done, PTAL.

}

var (
// RefsRegexp for string ref should be complied with description "Object names in the refs subdirectories MUST NOT include characters outside of the set of "A" to "Z", "a" to "z", "0" to "9", the hyphen -, the dot ., and the underscore _." at https://github.com/opencontainers/image-spec/blob/master/image-layout.md.
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for the URL anymore now that we're in the same repository. How about:

// RefsRegexp matches refs that meet the image-layout charset requirements.

or something?

Copy link
Author

Choose a reason for hiding this comment

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

OK, I will simplify the comment phrase.


var (
// RefsRegexp for string ref should be complied with description "Object names in the refs subdirectories MUST NOT include characters outside of the set of "A" to "Z", "a" to "z", "0" to "9", the hyphen -, the dot ., and the underscore _." at https://github.com/opencontainers/image-spec/blob/master/image-layout.md.
RefsRegexp = regexp.MustCompile(`^[a-zA-Z0-9\-\.\_]+?$`)
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't have to backslash-escape hyphen, period, or underscore inside your brackets. And I'm not sure why you have the ?. So I think this should be something like:

RefsRegexp = regexp.MustCompile(`^[a-zA-Z0-9-._]+$`)

Copy link
Author

Choose a reason for hiding this comment

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

I agree with you.

@wking
Copy link
Contributor

wking commented Oct 28, 2016

Cross-referencing opencontainers/image-tools#61.

it will be used in 'image-tools' or other project for checking regular
expression of 'refs', which writes as "Object names in the refs
subdirectories MUST NOT include characters outside of the set of "A" to
"Z", "a" to "z", "0" to "9", the hyphen -, the dot ., and the underscore
_." at https://github.com/opencontainers/image-spec/blob/master/image-layout.md

Signed-off-by: Ling FaKe <[email protected]>
@TrumanLing
Copy link
Author

@wking updated it, PTAL.

@wking
Copy link
Contributor

wking commented Oct 29, 2016 via email

@philips
Copy link
Contributor

philips commented Nov 16, 2016

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

jonboulle commented Nov 21, 2016

lgtm

Approved with PullApprove

@jonboulle jonboulle merged commit d4ecfe7 into opencontainers:master Nov 21, 2016
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.

4 participants