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

fix(run): reorder oclif sorted args #2976

Merged
merged 6 commits into from
Aug 15, 2024
Merged

fix(run): reorder oclif sorted args #2976

merged 6 commits into from
Aug 15, 2024

Conversation

zwhitfield3
Copy link
Contributor

Description

Work Item

This PR reorders the argv elements in a new array using the order that the user inputted. Since this updated logic needs to be applied to both heroku run and heroku local:run, the logic has been refactored into a utility function.

Testing

NOTE: I recommend commenting out the commands' functionality after consuming the args as you can see the bug without having to spin up a dyno.

  1. Pull down branch
  2. yarn it up
  3. Log userArgvInputOrder and argv downstream in both commands
  4. Run the commands with additional args like so:
    ./bin/run run -a testing-deploy -- ./print-args.sh --flag1 val1 --flag1 val2
  5. Confirm that argv maintains the sorted array due to the oclif dependency. (This is where the bug should be noticeable)
    example: argv [ './print-args.sh', '--flag1', '--flag1', 'val1', 'val2' ]
  6. Confirm that userArgvInputOrder reorders the array to maintain the same order you used when executing the command.
    example: userArgvInputOrder [ './print-args.sh', '--flag1', 'val1', '--flag1', 'val2' ]

@zwhitfield3 zwhitfield3 marked this pull request as ready for review August 14, 2024 06:23
@zwhitfield3 zwhitfield3 requested a review from a team as a code owner August 14, 2024 06:23
@zwhitfield3 zwhitfield3 merged commit aa560d3 into main Aug 15, 2024
8 checks passed
@zwhitfield3 zwhitfield3 deleted the zw/fix-arg-order branch August 15, 2024 19:30
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