This repository has been archived by the owner on Aug 1, 2024. It is now read-only.
feat: add go workshop #636
Merged
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
RomainMuller
approved these changes
Aug 23, 2022
4 tasks
|
Addressed these comments here and in our templates aws/aws-cdk#21728 |
mergify bot
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Aug 24, 2022
These changes were brought up in a [review of the Go workshop](aws-samples/aws-cdk-intro-workshop#636) - update Go version - close jsii process - remove unneeded `assertions` alias ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
RomainMuller
previously approved these changes
Aug 25, 2022
Some style fixes... Running `go fmt` and `goimports` to ensure code looks canonical. Changed branch names from `master` to `main` everywhere. Fixed a couple of missed edits, etc...⚠️ This is a PR on top of #636 --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
RomainMuller
previously approved these changes
Aug 26, 2022
indrora
suggested changes
Aug 26, 2022
There was a problem hiding this comment.
Quality of life things:
- Across the workshop (not required by this PR!) we should work on wording things a little clearer.
- I want alt-text. There's 36 spots of it.
Code wise I think there's no major nitpicks except the occasional placement of an interface reference in a struct declaration (which is ignored by the compiler, for various reasons).
I'm going to hold on alt-text though. Doesn't take much but we can port it around for other portions of the workshop.
| } | ||
|
|
||
| type cdkWorkshopStack struct { | ||
| awscdk.Stack |
There was a problem hiding this comment.
This isn't really the right place for this? I don't know if this is auto-generated or not but you don't typically place interface declarations inside a struct.
There was a problem hiding this comment.
This is actually what Romain suggested to me when I was struggling to figure out how to "extend" a "class". The way this works makes sense to me within my limited experience of the language, what would you suggest as an alternative?
workshop/content/60-go/70-advanced-topics/200-pipelines/3000-new-pipeline.md
Show resolved
Hide resolved
workshop/content/60-go/70-advanced-topics/200-pipelines/4000-build-stage.md
Show resolved
Hide resolved
workshop/content/60-go/70-advanced-topics/200-pipelines/5000-test-actions.md
Show resolved
Hide resolved
workshop/content/60-go/70-advanced-topics/200-pipelines/5000-test-actions.md
Show resolved
Hide resolved
indrora
approved these changes
Aug 29, 2022
josephedward
pushed a commit
to josephedward/aws-cdk
that referenced
this pull request
Aug 30, 2022
These changes were brought up in a [review of the Go workshop](aws-samples/aws-cdk-intro-workshop#636) - update Go version - close jsii process - remove unneeded `assertions` alias ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Creates workshop for Go
Still need to add advanced topic sections for testing and pipelines
also fixes #275
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.