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

Move parsing and name validation to cdi/parse package #130

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

elezar
Copy link
Contributor

@elezar elezar commented Mar 21, 2023

This should allow golang dependencies to be better managed.

See docker/cli#4084 (review)

@elezar elezar requested a review from klihub March 21, 2023 20:17
@elezar elezar force-pushed the better-is-qualified-name branch 2 times, most recently from a709266 to 8ea61a7 Compare March 21, 2023 20:27
@elezar elezar force-pushed the better-is-qualified-name branch 3 times, most recently from 35799bc to 2fb1ea1 Compare March 21, 2023 21:06
@elezar elezar requested a review from bart0sh March 22, 2023 10:39
}

// ParseQualifiedName splits a qualified name into device vendor, class,
// and name. If the device fails to parse as a qualified name, or if any
// of the split components fail to pass syntax validation, vendor and
// class are returned as empty, together with the verbatim input as the
// name and an error describing the reason for failure.
//
// Deprecated: use parser.Interface.ParseQualifiedName instead
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be use parser.ParseQualifiedName without .Interface. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

}

// ParseDevice tries to split a device name into vendor, class, and name.
// If this fails, for instance in the case of unqualified device names,
// ParseDevice returns an empty vendor and class together with name set
// to the verbatim input.
//
// Deprecated: use parser.Interface.ParseDevice instead
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto here

// If parsing fails, an empty vendor and the class set to the
// verbatim input is returned.
//
// Deprecated: use parser.Interface.ParseQualifier instead
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

Copy link
Contributor

@klihub klihub left a comment

Choose a reason for hiding this comment

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

I have 3 small nits related to doc strings. Otherwise LGTM.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar elezar requested a review from klihub March 24, 2023 13:44
@elezar
Copy link
Contributor Author

elezar commented Mar 24, 2023

I have 3 small nits related to doc strings. Otherwise LGTM.

Thanks. Missed updating the comments in my various iterations.

@elezar
Copy link
Contributor Author

elezar commented Mar 28, 2023

@klihub thanks for the review. Updated.

@bart0sh bart0sh merged commit f872caf into cncf-tags:main Mar 29, 2023
@elezar elezar deleted the better-is-qualified-name branch March 29, 2023 13:11
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.

3 participants