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

Flowey: Use explicit parameter names instead of auto generated. #606

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

benhillis
Copy link
Member

This change moves from auto-generated parameter names to requiring the caller to specify a variable name. This makes for easier to read generated pipeline files and allows for better behavior if another pipeline wants to use queue-time variables to control pipeline behavior.

@benhillis benhillis requested review from a team as code owners January 2, 2025 18:16
@benhillis benhillis requested a review from Copilot January 2, 2025 18:17

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 8 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • .github/workflows/openvmm-ci.yaml: Evaluated as low risk
  • flowey/flowey_cli/src/pipeline_resolver/direct_run.rs: Evaluated as low risk
  • flowey/flowey_cli/src/pipeline_resolver/generic.rs: Evaluated as low risk
Comments suppressed due to low confidence (4)

flowey/flowey_core/src/pipeline.rs:860

  • The new behavior of requiring explicit parameter names should be covered by tests to ensure it works as expected.
let param = self.pipeline.new_parameter_bool(name, description, default);

flowey/flowey_core/src/pipeline.rs:877

  • The new behavior of requiring explicit parameter names should be covered by tests to ensure it works as expected.
new_parameter_num(name, description, default, possible_values);

flowey/flowey_core/src/pipeline.rs:894

  • The new behavior of requiring explicit parameter names should be covered by tests to ensure it works as expected.
new_parameter_string(name, description, default, possible_values);

flowey/flowey_cli/src/pipeline_resolver/github_yaml/mod.rs:368

  • Ensure that there are tests covering the new behavior of using explicit parameter names instead of auto-generated ones.
let name = parameters[*pipeline_param_idx].name();
@benhillis benhillis force-pushed the user/benhill/pipeline_param_names branch from 11e7000 to bb1c8a5 Compare January 8, 2025 18:24
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.

3 participants