Handle field name overlaps #11262
Merged
Handle field name overlaps #11262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changelog[uncommitted] (2022-11-07)Bug Fixes
|
iwahbe
force-pushed
the
iwahbe/11251/allow-elementType-name-conflicts
branch
from
c4f295e
to
91c6598
Compare
lblackstone
approved these changes
Nov 4, 2022
AaronFriel
reviewed
Nov 4, 2022
pkg/codegen/go/utilities.go
Outdated
Comment on lines
152
to
172
| 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.
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.
We could, I just thought that this looked more natural. Its inline with how we deal with *schema.Object and *schema.Resource name conflicts.
iwahbe
force-pushed
the
iwahbe/11251/allow-elementType-name-conflicts
branch
from
91c6598
to
7726fb2
Compare
|
bors r+ |
|
Build failed: |
Frassle
reviewed
Nov 5, 2022
| // resource. | ||
| func isReservedResourceField(resourceName, s string) bool { | ||
| switch s { | ||
| case "ID", "URN", "GetProvider", "ElementType": |
There was a problem hiding this comment.
Are there any Go reserved words we should be checking here as well?
|
bors retry |
|
Already running a review |
kpitzen
force-pushed
the
iwahbe/11251/allow-elementType-name-conflicts
branch
from
7726fb2
to
5142d94
Compare
|
Canceled. |
|
bors retry |
|
Canceled. |
lblackstone
approved these changes
Nov 7, 2022
|
bors merge |
|
bors retry |
|
Already running a review |
|
Build failed: |
|
bors retry |
|
Build failed: |
|
bors retry |
|
Build succeeded: |
bors bot
added a commit
that referenced
this pull request
Nov 8, 2022
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>
bors bot
added a commit
that referenced
this pull request
Nov 8, 2022
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #11251