Skip to content

fix(angryjet): rename helpers to convert#100

Merged
jbw976 merged 2 commits intocrossplane:masterfrom
Duologic:duologic/fix_helpers_convert
Apr 24, 2025
Merged

fix(angryjet): rename helpers to convert#100
jbw976 merged 2 commits intocrossplane:masterfrom
Duologic:duologic/fix_helpers_convert

Conversation

@Duologic
Copy link
Copy Markdown
Contributor

@Duologic Duologic commented Apr 23, 2025

Description of your changes

Follow up on #91, this rename was forgotten there.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

I've been running this for generating https://github.com/grafana/crossplane-provider-grafana/

I did have to update the generated code intermittently as I couldn't run go mod tidy or make generate because of this rename. Not sure if qualifies as a breaking change.

Signed-off-by: Duologic <jeroen@simplistic.be>
@Duologic Duologic force-pushed the duologic/fix_helpers_convert branch from 0abf9ec to 6e507c9 Compare April 23, 2025 09:33
@Duologic
Copy link
Copy Markdown
Contributor Author

I'm getting a lint error locally:

11:37:49 [ .. ] golangci-lint
WARN [runner] Can't run linter goanalysis_metalinter: buildssa: failed to load package ast: could not load export data: internal error in importing "go/ast" (unsupported version: 2); please report an issue
ERRO Running error: can't run linter goanalysis_metalinter
buildssa: failed to load package ast: could not load export data: internal error in importing "go/ast" (unsupported version: 2); please report an issue
11:37:51 [FAIL]

Not sure how to solve that.

@Duologic
Copy link
Copy Markdown
Contributor Author

Updating golangci-lint seems to help...

Copy link
Copy Markdown
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution @Duologic! What do you think about updating ALL the mentions of helper while we're at it? 😂

Comment thread cmd/angryjet/main.go

methods := method.Set{
"ResolveReferences": method.NewResolveReferences(types.NewTraverser(comm), receiver, ClientImport, ReferenceImport, HelperImport, PtrImport),
"ResolveReferences": method.NewResolveReferences(types.NewTraverser(comm), receiver, ClientImport, ReferenceImport, ConvertImport, PtrImport),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great, I'll do that. Interesting that this isn't catched by linting or another CI test. 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

awesome @Duologic - just drop another message here when you're done and we'll take another look!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jbw976 done, grepped for 'helper' and replaced what I found.

Signed-off-by: Duologic <jeroen@simplistic.be>
@Duologic Duologic force-pushed the duologic/fix_helpers_convert branch from 9ae78c9 to d4a4d50 Compare April 24, 2025 12:57
@Duologic Duologic requested a review from jbw976 April 24, 2025 12:58
Copy link
Copy Markdown
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

cool, this looks good to me @Duologic, thanks for the follow up! 🙇‍♂️

@jbw976 jbw976 merged commit de0e510 into crossplane:master Apr 24, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants