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(plus-17): L2 default child #389

Merged
merged 16 commits into from
Nov 18, 2020
Merged

fix(plus-17): L2 default child #389

merged 16 commits into from
Nov 18, 2020

Conversation

iliapolo
Copy link
Member

@iliapolo iliapolo commented Nov 18, 2020

All L1 children are now called Resource to mark them as the default child of the L2.

BREAKING CHANGE: All L2 resource names will undergo a name change (e.g test-chart-config-configmap-233db8e7 -> test-chart-config-c3f7d3c0)

Resolves #373

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@iliapolo iliapolo changed the title Epolon/l2 default child fix: L2 default child Nov 18, 2020
@iliapolo iliapolo changed the title fix: L2 default child fix(plus-17): L2 default child Nov 18, 2020
@iliapolo iliapolo requested a review from eladb November 18, 2020 14:30
packages/cdk8s-plus-17/src/config-map.ts Show resolved Hide resolved

const chart = Testing.chart();

expect(Node.of(new ConfigMap(chart, 'ConfigMap')).defaultChild).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be better to assert instanceof KubeConfigMap or at least ApiObject.

BTW, should we rename ApiObject to KubeObject?

@@ -142,6 +122,26 @@ function omitDuplicates(value: string, index: number, components: string[]) {
return value !== components[index-1];
}

function omitDefaultChild(value: string, _: number, __: string[]) {
return value.toLowerCase() !== 'resource' && value.toLowerCase() !== 'default';
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it is actually case sensitive in constructs.

packages/cdk8s/src/names.ts Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Nov 18, 2020

Your pull request will be updated and merged automatically (do not update manually).

@mergify
Copy link
Contributor

mergify bot commented Nov 18, 2020

Your pull request will be updated and merged automatically (do not update manually).

@mergify mergify bot merged commit a8337e8 into master Nov 18, 2020
@mergify mergify bot deleted the epolon/l2-default-child branch November 18, 2020 15:49
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.

plus: ensure all L2s have a defaultChild set up for them, ideally named "Resource"
2 participants