snok / install-poetry

Github action for installing and configuring Poetry
MIT License
573 stars 53 forks source link

WIP: Add ability to install plugins #69

Closed connortann closed 1 year ago

connortann commented 2 years ago

Closes #53

Adds ability to install plugins with:

- uses: snok/install-poetry@v1
  with:
    version: 1.2.0a2
    plugins: poetry-plugin-a, poetry-plugin-b

Might be worth adding a check that the Poetry version is at least 1.2 before trying to install plugins? I also note that version is not released yet, so possibly the syntax may change in future.

sondrelg commented 2 years ago

Do you think this is something we could add a test for, to verify that plugins are really installed @connortann? 🙂

connortann commented 2 years ago

Oh nice, I didn't realise you had a test suite in GitHub workflows, I didn't think to look there. Certainly, I'll have a go! I'll need to think about to verify which plugins are installed; will do some experimentation.

miigotu commented 2 years ago

That release is yanked, it causes issues with other packages that require poetry.

sondrelg commented 2 years ago

That release is yanked, it causes issues with other packages that require poetry.

The Poetry 1.2.0a2 release was yanked? I guess that won't impact the feature, but maybe we should use 1.2.0 for the docs example, even though it's not released.

Cheers for adding the test - that looks good to me @connortann 🙂 Would you like to add docs here as well? If you prefer I can add docs myself in a separate PR, just let me know 👍

connortann commented 2 years ago

Sure, I'll suggest adding something to the README. I also want to double-check the shell script I've added, so will set this PR as "WIP" for now.

Also, I noticed that the ShellCheck CI linter doesn't check the shell script within action.yml, so it would be nice to enable that if possible. Would it make sense for the bulk of the script within the action to call a main.sh ?

EDIT: another option could be to use a CI job to temporarily extract the shell scripts to a .sh file, before running ShellCheck

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sondrelg commented 2 years ago

I'd love to do this, but as far as I know (I hope I'm wrong), there's no way to bundle a main.sh script with the action. So if you called a script from action.yml that would break when run on an external repo. Dockerfile-based actions don't have this issue, since there you can just copy the relevant files into the container when building it.

Also wish there was something like this just with shellcheck and action.yml files 😛 If you can work out a way of getting this to work, I'd be very happy to accept it. I usually throw the code into a shell script, lint it, then copy it back.

connortann commented 2 years ago

I've attempted the migration to a main.sh on a separate PR #70 . I think it would be good to merge that one first; then I can rebase this PR accordingly.

EDIT: rebased this PR on top of main and fixed the conflicts. Nice having the ShellCheck linter working, it helped me avoid a few bash style issues.

sondrelg commented 1 year ago

Closing this for now @connortann, but feel free to reopen. Think most of the blockers are resolved now.