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.

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

Proposal - Make analyze variables and functions public #115

Open
rikatz opened this issue Jul 30, 2024 · 1 comment
Open

Proposal - Make analyze variables and functions public #115

rikatz opened this issue Jul 30, 2024 · 1 comment

Comments

@rikatz
Copy link

rikatz commented Jul 30, 2024

Is your feature request related to a problem? Please describe

This is a feature request to make analyzer.go constants public and change the generator to allow consuming these constants

Describe the solution you'd like

Using generators to create analyzers is great! This means I can have my own third party, generated consistently from official code (see https://github.com/kubernetes/ingress-nginx/tree/feature-go-crossplane/internal/ingress/controller/template/crossplane/extramodules)

The problem is that the const directives (https://github.com/kubernetes/ingress-nginx/blob/feature-go-crossplane/internal/ingress/controller/template/crossplane/extramodules/analyze.go#L30) are private, meaning I had to partially copy the file to the same place of the generated files.

As a proposal:

  • Make these consts public
  • Change the generator, so you can tell if it should get these const from the same dir (meaning NGINX Inc is probably running it to port to nginx-go-crossplane) or I am a third party generating an analyzer for my module, and not needing to copy partially these directives

Describe alternatives you've considered

See the file above on the alternative approach used

Additional context

Add any other context or screenshots about the feature request here.

@ornj
Copy link
Member

ornj commented Jul 30, 2024

@rikatz 100% agree. With the inclusion of the generators and an API evolving to enable the use case you've mentioned I agree that these constants should be exported and well documented.

As far as discovering constants, I'm not sure what that might look like. The constant values that currently exist in crossplane differ from the actual values you will find in the nginx source. Some times the value is different between crossplane and nginx, some times they do not exist in nginx source code at all.

Changes would need to be made to the analyzer code I think to make use of any new constants defined by the generator.

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

No branches or pull requests

2 participants