Accept additional options to pass to poetry

[mario-bermonti]

Accept additional options from the user that are directly passed to poetry when installing it.

This will allow to install directly from github or use any of the other options accepted by poetry's installer script.

This closes #59.

[mario-bermonti]

I added 2 tests, one for each "branch" of the if statement block (version specified vs version unspecified by the user).

I would like your feedback to know if this is the right approach. If it is, I would add this feature to the docs.

Even though the tests passed, I got the following message for the 2 tests added:
Warning: Unexpected input(s) 'additional-options', valid inputs are ['version', 'virtualenvs-create', 'virtualenvs-in-project', 'virtualenvs-path', 'installer-parallel', 'other-options']

This is expected until the changes are merged or am I missing something?

[sondrelg]

The warning output you're seeing is likely due to the fact that you've specified additional-options in your tests, but called it other-options in the action.yml. You also passed in other_options which I think is incorrect. I think this issue should be resolved by the suggestions I've added.

Otherwise good job, I think this is the way to go!

In terms of finalizing the PR now, I have one request: would you mind fixing the tests and making them fail before applying the suggestions I added? The fact that tests are currently passing indicates to me that we're not actually testing anything useful yet. Ideally we want tests to fail now, then when we fix the argument errors the tests should pass. Does that make sense?

[sondrelg]

Suggested change

	additional-options: --force
	installation-arguments: --force

[sondrelg]

Suggested change

	additional-options: --git https://github.com/python-poetry/poetry.git@master
	installation-arguments: --git https://github.com/python-poetry/poetry.git@master

[sondrelg]

I think the easiest way to make the tests fail here is to use the --git argument in both tests to install a specific version, then check the version installed. For the test to have value though, we cannot specify 1.1.7 and check that 1.1.7 is installed.

[mario-bermonti]

It makes complete sense, @sondrelg. Sorry for the naming mistakes. That was sloppy of me.

I checked and the —git argument takes precedence over —version in the poetry installer. We can pass both and make sure it installs the version that corresponds to the git hash specified with the —git command.

I’m going to work in this soon and report back.

[sondrelg]

One more thing btw: would it make sense to cut this whitespace for these tests? The whitespace for the final test seems OK, but here it's a little confusing 🙂

[sondrelg]

Think we might be able to remove these comments now, but otherwise looks very good!

If tests pass I'll release a new version later tonight 🎉

[sondrelg]

Is this still true?

[mario-bermonti]

Not anymore. Cleaned!

[mario-bermonti]

I had to submit these changes that just continue to rename the additional-options to installation-arguments in a different commit (dded203) because I made changes to these lines and the suggestions were flagged as outdated.

[mario-bermonti]

I am going to need guidance with the documentation. How would you suggest this feature be documented, @sondrelg?

[sondrelg]

I think we could add the option in the first two code blocks in the Readme, then add a sentence and an example about how to specify installer arguments at the end of the 'Defaults' section - just below (or above if you think it's better) the stuff about experimental settings 👏

[sondrelg]

I pushed final small edit to the readme, but really happy with this feature @mario-bermonti. Thanks a lot 👏

[mario-bermonti]

Thank you for a great experience, @sondrelg.