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
[sdk/go] chore: update some notes and make some lint #10984
Conversation
about error string: error string should not be capitalized or end with punctuation mark
Changelog[uncommitted] (2022-10-11)Miscellaneous
|
Co-authored-by: Fraser Waters <frassle@gmail.com>
Co-authored-by: Fraser Waters <frassle@gmail.com>
bors merge |
10984: [sdk/go] chore: update some notes and make some lint r=Frassle a=asjdf I updated some notes, updated the deprecated functions, and make some lint. Reason for error string change: error string should not be capitalized or end with a punctuation mark. According to the p/p [readme](https://github.com/pulumi/pulumi#languages), I think it is time to remove some deprecated function like ioutil.ReadFile. #6703 ## Checklist - [x] I have run tests - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change - [x] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: 杨成锴 <homeboyc@foxmail.com>
Build failed: |
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.
Made a number of suggestions and requested changes.
The change to types_builtin.go
seems likely to be undone next time the file is generated.
@asjdf Thanks for the contribution, sorry for all the late breaking suggestions here, but it felt appropriate to put them in one place. @Frassle if any of these suggestions seem off, let me know. I think the change to Tomorrow I plan to add a |
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.
Requested a change on the types_builtin.go in particular. The remainder of the suggestions I made could be applied in a follow up PR, but this is a convenient place to make them.
@asjdf thank you for taking the time to open this pr!! ❤️ |
Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
Thank you all for taking the time to review my pr. ❤ |
I think you are right, so I revert the changes in types_builtin.go. I'm sorry for my hasty changes in types_builtin.go. |
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.
Looks good to me, thank you!
bors merge
10984: [sdk/go] chore: update some notes and make some lint r=AaronFriel a=asjdf I updated some notes, updated the deprecated functions, and make some lint. Reason for error string change: error string should not be capitalized or end with a punctuation mark. According to the p/p [readme](https://github.com/pulumi/pulumi#languages), I think it is time to remove some deprecated function like ioutil.ReadFile. #6703 ## Checklist - [x] I have run tests - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change - [x] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: 杨成锴 <homeboyc@foxmail.com>
Build failed: |
@asjdf I'll need to take another pass, as it looks tests are failing with an error that points to a string formatting change:
|
I'm really sorry that this pr has taken up so much of your time. Thank you very much for your help.❤️ |
@asjdf thank you for taking the time to make this contribution! It's long overdue cleanup to be consistent with Go's style guide. I've started a new PR #11002 based off this branch to see why those some of those checks failed. It looks like the main issue is Go's behavior and requirements for pointer vs value receivers with marshaling. I found this Go playground entry (the same as linked above) which shows that Go's behavior here seems inconsistent. I learned the hard way to be careful changing methods from pointer receivers to value receivers early on. |
Thank you for your help and looks like I can turn this pr off? |
@asjdf if you want to click "Allow edits from maintainers", I can push my changes back onto this branch and merge. |
Closing in favor of #11002 |
11002: Test changes to #10984 r=AaronFriel a=AaronFriel This is a rebase and squash of #10984 with an additional commit added to satisfy `make lint` and revert a few changes to methods to use value receivers, where the original PR altered marshaling/unmarshaling behavior. Co-authored-by: 杨成锴 <homeboyc@foxmail.com> Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
11002: Reimplement #10984 r=AaronFriel a=AaronFriel This is a rebase and squash of #10984 with an additional commit added to satisfy `make lint` and revert a few changes to methods to use value receivers, where the original PR altered marshaling/unmarshaling behavior. 11031: prepare for next release (v3.43.1) r=AaronFriel a=pulumi-bot 11059: (pulumi-bot) Synced file(s) with pulumi/pulumi-yaml r=aq17 a=pulumi-bot Synced local file(s) with [pulumi/pulumi-yaml](https://github.com/pulumi/pulumi-yaml). This PR syncs changes to the codegen'd PCL files from the latest `pulumi/yaml` release <details> <summary>Changed files</summary> <ul> <li>Created local directory <code>pkg/codegen/testing/test/testdata/transpiled_examples</code> and copied all sub files/folders from remote directory <code>pkg/tests/transpiled_examples</code></li> </ul> </details> --- This PR was created automatically by the [repo-file-sync-action](https://github.com/BetaHuhn/repo-file-sync-action) workflow run [#3269786749](https://github.com/pulumi/pulumi-yaml/actions/runs/3269786749) Co-authored-by: 杨成锴 <homeboyc@foxmail.com> Co-authored-by: Aaron Friel <mayreply@aaronfriel.com> Co-authored-by: pulumi-bot <null> Co-authored-by: Alex Qiu <aqiu@pulumi.com>
I updated some notes, updated the deprecated functions, and make some lint.
Reason for error string change: error string should not be capitalized or end with a punctuation mark.
According to the p/p readme, I think it is time to remove some deprecated function like ioutil.ReadFile. #6703
Checklist
make changelog
and committed thechangelog/pending/<file>
documenting my change