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
Handle field name overlaps #11262
Handle field name overlaps #11262
Conversation
Changelog[uncommitted] (2022-11-07)Bug Fixes
|
c4f295e
to
91c6598
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- thanks for the quick fix!
pkg/codegen/go/utilities.go
Outdated
switch typ := codegen.UnwrapType(p.Type).(type) { | ||
case *schema.EnumType: | ||
postfix = "Enum" | ||
case *schema.ObjectType: | ||
postfix = "Type" | ||
case *schema.ResourceType: | ||
postfix = "Resource" | ||
default: | ||
if schema.IsPrimitiveType(typ) { | ||
switch typ { | ||
case schema.StringType: | ||
postfix = "Str" | ||
case schema.BoolType: | ||
postfix = "Bool" | ||
case schema.IntType: | ||
postfix = "Int" | ||
case schema.NumberType: | ||
postfix = "Num" | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we simplify this to adding a _
suffix? Each object type can only have a single field named ElementType
for example, I don't see what we gain by adding type info in the field name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, I just thought that this looked more natural. Its inline with how we deal with *schema.Object
and *schema.Resource
name conflicts.
91c6598
to
7726fb2
Compare
bors r+ |
Build failed: |
// resource. | ||
func isReservedResourceField(resourceName, s string) bool { | ||
switch s { | ||
case "ID", "URN", "GetProvider", "ElementType": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any Go reserved words we should be checking here as well?
bors retry |
Already running a review |
7726fb2
to
5142d94
Compare
Canceled. |
bors retry |
Canceled. |
bors merge |
bors retry |
Already running a review |
Build failed: |
bors retry |
Build failed: |
bors retry |
Build succeeded: |
11277: ci: block non-linear merges r=AaronFriel a=AaronFriel This ensures commit history is linear, enabling customer-owned forks of the pulumi CLI to more easily maintain their fork. Reverse merges into PR branches result in a more complex process for them and for us. To test this PR, I based my PR branch off an older commit from the target branch and added a merge commit. That resulted in the check failing: https://github.com/pulumi/pulumi/actions/runs/3416559764/jobs/5686840393 > Checking merge commit efb7be0 for non-linear history > Main branch parent is: 83c9dfc Merge #11262 > PR branch parents are d9460b9 > Checking: d9460b9 Merge remote-tracking branch 'origin/master' into friel/block-reverse-merge > Error: Non-linear history, PR contains a merge d9460b9. Remove this by rebasing on the target. > Error: Detected non-linear history. > Error: Process completed with exit code 1. Fixes #10903 The script used by this lint contains several "tests" and useful diagnostics. Example outputs: ```shell $ ./scripts/git-linear-history-check.sh f033d9d Checking merge commit f033d9d for non-linear history Main branch parent is: 0797f29 Merge #10817 PR branch parents are 110dd76 ffbb03c cdf8f20 396650a Checking: 110dd76 ci: Pin yarn lockfile for security & dependency scanning Checking: ffbb03c ci: Build binary with .exe extension on Windows Checking: cdf8f20 ci: Remove several test skips, check if unnecessary Checking: 396650a ci: Re-enable Windows tests with temp dir ✅ Commit history is linear. ``` ```shell $ ./scripts/git-linear-history-check.sh 0f3e536 Checking merge commit 0f3e536 for non-linear history Main branch parent is: bc704af Merge #10703 #10717 PR branch parents are 9065d7c 22f2989 9f5ec4a Checking: 9065d7c refactored defaultServiceLoop into its own method ::error::Non-linear history, PR contains a merge fa09da6. Remove this by rebasing on the target. ::error::Non-linear history, PR contains a merge 536f3d6. Remove this by rebasing on the target. Checking: 22f2989 ci: Fix package parallelism assignment Checking: 9f5ec4a Add missing `ProgramTestOptions` overrides in `With` ::error::Detected non-linear history. ``` The `::error::` messages should appear in GitHub Actions logs at the top level (the workflow level) as well as in the detailed output of the action. Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
11277: ci: block non-linear merges r=AaronFriel a=AaronFriel This ensures commit history is linear, enabling customer-owned forks of the pulumi CLI to more easily maintain their fork. Reverse merges into PR branches result in a more complex process for them and for us. To test this PR, I based my PR branch off an older commit from the target branch and added a merge commit. That resulted in the check failing: https://github.com/pulumi/pulumi/actions/runs/3416559764/jobs/5686840393 > Checking merge commit efb7be0 for non-linear history > Main branch parent is: 83c9dfc Merge #11262 > PR branch parents are d9460b9 > Checking: d9460b9 Merge remote-tracking branch 'origin/master' into friel/block-reverse-merge > Error: Non-linear history, PR contains a merge d9460b9. Remove this by rebasing on the target. > Error: Detected non-linear history. > Error: Process completed with exit code 1. Fixes #10903 The script used by this lint contains several "tests" and useful diagnostics. Example outputs: ```shell $ ./scripts/git-linear-history-check.sh f033d9d Checking merge commit f033d9d for non-linear history Main branch parent is: 0797f29 Merge #10817 PR branch parents are 110dd76 ffbb03c cdf8f20 396650a Checking: 110dd76 ci: Pin yarn lockfile for security & dependency scanning Checking: ffbb03c ci: Build binary with .exe extension on Windows Checking: cdf8f20 ci: Remove several test skips, check if unnecessary Checking: 396650a ci: Re-enable Windows tests with temp dir ✅ Commit history is linear. ``` ```shell $ ./scripts/git-linear-history-check.sh 0f3e536 Checking merge commit 0f3e536 for non-linear history Main branch parent is: bc704af Merge #10703 #10717 PR branch parents are 9065d7c 22f2989 9f5ec4a Checking: 9065d7c refactored defaultServiceLoop into its own method ::error::Non-linear history, PR contains a merge fa09da6. Remove this by rebasing on the target. ::error::Non-linear history, PR contains a merge 536f3d6. Remove this by rebasing on the target. Checking: 22f2989 ci: Fix package parallelism assignment Checking: 9f5ec4a Add missing `ProgramTestOptions` overrides in `With` ::error::Detected non-linear history. ``` The `::error::` messages should appear in GitHub Actions logs at the top level (the workflow level) as well as in the detailed output of the action. Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
Fixes #11251