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

kintsugi: Fix passing options from install-driver to driver. #100

Merged
merged 1 commit into from
Apr 9, 2023

Conversation

byohay
Copy link
Collaborator

@byohay byohay commented Apr 9, 2023

No description provided.

Copy link
Member

@ashdnazg ashdnazg left a comment

Choose a reason for hiding this comment

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

LGTM

@byohay byohay force-pushed the feature/fix-pass-driver-parameters branch 4 times, most recently from 99b018f to 40e39cc Compare April 9, 2023 07:35
@@ -106,8 +104,9 @@ def create_install_driver_subcommand
end

def install_kintsugi_driver_globally(options)
joined_options = options.count > 1 ? " #{options.join(" ")}" : ""
Copy link
Member

@ashdnazg ashdnazg Apr 9, 2023

Choose a reason for hiding this comment

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

shouldn't this just be options.join(" ")?
at the moment it ignores options if it has just one element

Copy link
Collaborator Author

@byohay byohay Apr 9, 2023

Choose a reason for hiding this comment

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

oops, it's supposed to be options.count > 0. The problem is that when is there are no options a trailing space is added which writes the driver command incorrectly:
driver = "kintsugi driver %O %A %B %P "
However I can use .rstrip instead.

@byohay byohay force-pushed the feature/fix-pass-driver-parameters branch from 40e39cc to ffb905f Compare April 9, 2023 13:41
Copy link
Member

@ashdnazg ashdnazg left a comment

Choose a reason for hiding this comment

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

LGTM

@byohay byohay merged commit 13f01c9 into Lightricks:main Apr 9, 2023
1 check passed
@byohay byohay deleted the feature/fix-pass-driver-parameters branch April 9, 2023 13:48
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.

Invalid option error in install-driver when using driver-options
2 participants