Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use ReplaceNodeContent in persist for transformations #40

Merged
merged 4 commits into from
Jan 12, 2023
Merged

Use ReplaceNodeContent in persist for transformations #40

merged 4 commits into from
Jan 12, 2023

Conversation

gordon-klotho
Copy link
Contributor

Utilize the changes in #20 to simplify persist plugin.

Standard checks

  • Unit tests: Updated unit tests
  • Docs: internal only
  • Backwards compatibility: No breaking changes

@gordon-klotho gordon-klotho changed the base branch from main to internal/source_file January 6, 2023 16:24
@gordon-klotho gordon-klotho changed the title Internal/inplace transform Use ReplaceNodeContent in persist for transformations Jan 6, 2023
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 6, 2023 21:10 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 9, 2023 15:57 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 10, 2023 17:02 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:08 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:08 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:08 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:08 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:08 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
@gordon-klotho gordon-klotho temporarily deployed to integ_test January 11, 2023 18:09 — with GitHub Actions Inactive
ghost
ghost approved these changes Jan 11, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice — I like how this reduces the boilerplate!

pkg/lang/javascript/plugin_persist.go
var doTransform func(original *core.SourceFile, modified *core.SourceFile, cap *core.Annotation, result *persistResult, unit *core.ExecutionUnit) (core.CloudResource, error)
var err error
var resource core.CloudResource
var err, runtimeErr, transformErr error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, but what if we instead had this be a multierr.Error, and then always appended each err to it (which I think is a noop if the err you're appending is nil?), and then at line 204-210 just checked if the multierr is non-empty? I think that would simplify things, at least to me; fewer vars to think about .

Definitely non-blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea, but when I tried it I realized the switch block relies on setting the resource var (= not := so it doesn't go out of scope) so I'd have to define these errors anyway.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never change, Go, never change. 🙃

@gordon-klotho
Change annotations to be a map with pointer values
95b467d 
Additionally, on reparse the annotation values are updated instead of replaced leaving the pointer values still valid
@gordon-klotho
Use ReplaceNodeContent in persist for transformations
0097bde
@gordon-klotho
Re-add InSourceOrder
71fb246
@gordon-klotho
Fix regression on typeorm
fb2273c 
expression is now a node and needs .Content
Base automatically changed from internal/source_file to main January 12, 2023 13:30
@gordon-klotho gordon-klotho merged commit b5820ea into main Jan 12, 2023
@gordon-klotho gordon-klotho deleted the internal/inplace_transform branch January 12, 2023 13:31
@github-actions
Copy link

Package Line Rate Health
github.com/klothoplatform/klotho/pkg/analytics 2% ❌
github.com/klothoplatform/klotho/pkg/annotation 24% ❌
github.com/klothoplatform/klotho/pkg/core 20% ❌
github.com/klothoplatform/klotho/pkg/env_var 82% ✔
github.com/klothoplatform/klotho/pkg/exec_unit 45% ❌
github.com/klothoplatform/klotho/pkg/infra/kubernetes 58% ➖
github.com/klothoplatform/klotho/pkg/infra/kubernetes/helm 52% ➖
github.com/klothoplatform/klotho/pkg/input 63% ➖
github.com/klothoplatform/klotho/pkg/lang 37% ❌
github.com/klothoplatform/klotho/pkg/lang/dockerfile 0% ❌
github.com/klothoplatform/klotho/pkg/lang/golang 9% ❌
github.com/klothoplatform/klotho/pkg/lang/javascript 47% ❌
github.com/klothoplatform/klotho/pkg/lang/python 60% ➖
github.com/klothoplatform/klotho/pkg/lang/yaml 0% ❌
github.com/klothoplatform/klotho/pkg/logging 7% ❌
github.com/klothoplatform/klotho/pkg/multierr 95% ✔
github.com/klothoplatform/klotho/pkg/provider/aws 60% ➖
github.com/klothoplatform/klotho/pkg/runtime 75% ✔
github.com/klothoplatform/klotho/pkg/static_unit 32% ❌
github.com/klothoplatform/klotho/pkg/validation 73% ➖
Summary 42% (3592 / 8582) ❌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@gordon-klotho