[multiple] Improve cifmw_project_dir group var name #3583
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
amartyasinha
force-pushed
the
improve_var_name
branch
from
95f3e9c to
e5f9b2d
Compare
|
zuul gate jobs were stuck for unknown reason. this force commit adds nothing, just meant to retrigger the jobs. |
|
recheck |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
Changing the variable to ensure all variable names are following the similar pattern. - Now, cifmw_project_dir will be ci_framework_repo_relative and cifmw_project_dir_absolute will be ci_framework_repo - Since the absolute values are used in the entire framework, that variable should not have extra keyword (absolute). - The reason to move from `dir` to `repo` is to match the pattern of other variables. Signed-off-by: Amartya Sinha <amsinha@redhat.com>
amartyasinha
force-pushed
the
improve_var_name
branch
from
e5f9b2d to
e25bf75
Compare
|
Merging blocked as gate job |
|
recheck |
| cifmw_project_dir: src/github.com/openstack-k8s-operators/ci-framework | ||
| cifmw_project_dir_absolute: "{{ ansible_user_dir }}/{{ cifmw_project_dir }}" | ||
| ci_framework_repo_relative: src/github.com/openstack-k8s-operators/ci-framework | ||
| ci_framework_repo: "{{ ansible_user_dir }}/{{ ci_framework_repo_relative }}" |
There was a problem hiding this comment.
nitpick: These names break from the pattern of prefixing these vars with cifmw_, maybe something like cifmw_framework_repo would be better? Open to disagreement or other suggestions, though!
| vars: | ||
| cifmw_build_containers_cleanup: true | ||
| cifmw_build_containers_config_file: "{{ cifmw_project_dir_absolute }}/roles/build_containers/files/containers.yaml" | ||
| cifmw_build_containers_config_file: "{{ ci_framework_repo }}/roles/build_containers/files/containers.yaml" |
There was a problem hiding this comment.
nitpick: While you're making edits here you could also fix the extra space on the right side of the {{ }}.
| hosts: all | ||
| vars: | ||
| cifmw_basedir: "{{ cifmw_project_dir_absolute }}" | ||
| cifmw_basedir: "{{ ci_framework_repo }}" |
There was a problem hiding this comment.
See other comment about removing an extra space.
3 participants
Changing the variable to ensure all variable names are following the similar pattern.
cifmw_project_dirwill beci_framework_repo_relativeandcifmw_project_dir_absolutewill beci_framework_repo.dirtorepois to match the pattern of other variables.Depends-On: openstack-k8s-operators/tcib#356