fix: Package name collisions #22
Conversation
Fixes: cdk8s-team/cdk8s#273 Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
|
@campionfellin Can you see if you can fix this but while you are at it? |
…on package names Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
|
|
||
| __all__ = [ | ||
| \\"__jsii_assembly__\\", | ||
| ] | ||
|
|
||
| publication.publish() | ||
| ", | ||
| "my_python_module/submodule/_jsii/[email protected]": " � �ko�H�����K���6��D��� ��� �TE�z����5�T����<]Ҫ���ݙ��Ύ��ih��p.;f��y�$�T(�� |
There was a problem hiding this comment.
This needs to be filtered out
There was a problem hiding this comment.
Yeah... not really sure why my macbook keeps trying to add them... will try to fix
This reverts commit 58e1784.
This reverts commit 824bb84.
Signed-off-by: campionfellin <[email protected]>
src/options.ts
Outdated
| * Package name | ||
| * @default - hash of the basepath to the module | ||
| */ | ||
| packageName?: string |
There was a problem hiding this comment.
I am not sure that's the correct name for this because users don't really interact with this value after code generation. Maybe moduleKey or something like that.
Docstring help needs to indicate that it should be project-unique.
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
src/compile.ts
Outdated
| @@ -66,7 +68,7 @@ export async function compile(workdir: string, options: Options) { | |||
| if (options.python) { | |||
| targets.python = { | |||
| distName: 'generated', | |||
| module: options.python.moduleName, | |||
| module: options.python.moduleName.replace(/-/g, '_'), | |||
There was a problem hiding this comment.
I am assuming java would have a similar issue. I wonder if the correct behavior here is to validate per the language constraints and throw an error so the user will be required to pass in a valid name for the module.
There was a problem hiding this comment.
Yeah! We can do some validation instead of just correcting it. I'd be worried in some cases though where the user isn't the one explicitly choosing what this value is. In some use-cases, like CRDs in cdk8s from the internet, they might not control the group.
Would we then add this correction behavior to all consumers?
There was a problem hiding this comment.
Maybe we can do a warn that lets the user know we're correcting it?
There was a problem hiding this comment.
I think the consumers of this library (eg cdk8s) should do the name mangling if needed. The constraints should be well documented
There was a problem hiding this comment.
Ok! I will add validation here then
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Signed-off-by: campionfellin <[email protected]>
Fixes: cdk8s-team/cdk8s#273
Signed-off-by: campionfellin [email protected]
Issue #, if available:
cdk8s-team/cdk8s#273
Description of changes:
Let consumers pass in a
packageNameto be used by JSII. Otherwise fall back togenerated.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.