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

Fix various usability problems in yaml.* #638

Merged
merged 5 commits into from
Jul 22, 2019
Merged

Conversation

hausdorff
Copy link
Contributor

@hausdorff hausdorff commented Jul 13, 2019

Fixes #448.
Fixes #502.
Fixes #501.

@hausdorff hausdorff changed the title Allow yaml.ConfigGroup to take URLs as argument Fix various usability problems in yaml.* Jul 13, 2019
@hausdorff hausdorff force-pushed the hausdorff/cg-urls branch 3 times, most recently from c6d600c to 52c1308 Compare July 15, 2019 23:47
Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

Can we add tests for the fixes + new features?

}
}

/** @ignore */ function isUrl(s: string): boolean {
return s.startsWith("http://") || s.startsWith("https://")
Copy link
Member

Choose a reason for hiding this comment

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

Should we be instead attempt to parse a URL with https://nodejs.org/api/url.html#url_class_url and return true if parsing succeeds? This would allow us to handle file URLs, etc.

Copy link
Member

Choose a reason for hiding this comment

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

(I do see that what you've written matches the prior behavior of ConfigFile, so no urgency here)

The fields of the Kubernetes core resource types are tagged with Go's
`"json:"` serialization metadata. They expect things like JavaScript's
`Date` to be serialized as string.

Fixes #501.
@lblackstone lblackstone moved this from backlog to in progress in Q3 Kubernetes Jul 22, 2019
@lblackstone lblackstone merged commit 2ed523a into master Jul 22, 2019
@pulumi-bot pulumi-bot deleted the hausdorff/cg-urls branch July 22, 2019 20:41
@hausdorff hausdorff mentioned this pull request Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Q3 Kubernetes
in progress
3 participants