search

bartdorlandt / ansible-role-poetry

1 stars 2 forks source link

Matching Poetry parameter default values #2

Closed hongbo-miao closed 1 year ago

hongbo-miao commented 1 year ago

Currently I saw in readme, it has two parameters which are set to true`, however, the default values in Poetry are different.

Just a suggestion, it would be great to match Poetry parameter default values. So that people won't be surprised when not pass any parameters by default. Thanks! 😃

bartdorlandt commented 1 year ago

Feel free to do a PR 😉👍

On Wed, 9 Aug 2023 at 21:12 Hongbo Miao @.***> wrote:

Currently I saw in readme, it has two parameters which are set to true`, however, the default values in Poetry are different.

Just a suggestion, it would be great to be consistent with Poetry by default. So that people (my coworkers just did 😅) won't be surprised when not pass any parameters by default. Thanks! 😃

— Reply to this email directly, view it on GitHub https://github.com/bartdorlandt/ansible-role-poetry/issues/2, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHT3H6EGXJ4YMXDXVYMA3LXUQ7SBANCNFSM6AAAAAA3KZYMCE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

hongbo-miao commented 1 year ago

Hi @bartdorlandt I tried to contribute, however as I am still new to Ansible. I am not sure how to handle below situation correctly. Need some help from you, thanks! ☺️

Here is what I found:

virtualenvs.in-project can have three values null, true, false, and by default is null.

(Note null is what I got from poetry config virtualenvs.in-project, while the official document is using None)

So actually based on current code

https://github.com/bartdorlandt/ansible-role-poetry/blob/163188619d6a12d327c795655bb622e26aac8bcd/tasks/main.yml#L18-L26

  1. When pass virtualenvs_inproject: false in this role, it actually preserve default null instead of setting to false:

    parallels@ubuntu-linux-22-04-desktop:~/.local/bin$ poetry config virtualenvs.in-project
null

  2. When pass virtualenvs_inproject: true in this role, it will set to true.

  3. Currently, there is no way to set to false.

I was hoping to change to default null, so updated code would be like

 - name: Check Poetry virtualenvs.in-project config 
   ansible.builtin.command: poetry config virtualenvs.in-project 
   register: poetry_in_project 
   changed_when: ??? # not sure here 
   when: virtualenvs_inproject 

 - name: Configure Poetry virtualenvs.in-project 
   ansible.builtin.command: poetry config virtualenvs.in-project {{ virtualenvs_inproject }}  # not sure if I did correct
   when: virtualenvs_inproject != poetry_in_project.stdout

However, I have two questions inside. Sorry still too new to Ansible...

bartdorlandt commented 1 year ago

Hi, Surprises do happen when not reading ;)

thank you for the suggestion anyways, it is a valid one and I've changed it accordingly.

Cheers, Bart

On Wed, Aug 9, 2023 at 9:12 PM Hongbo Miao @.***> wrote:

Currently I saw in readme, it has two parameters which are set to true`, however, the default values in Poetry are different.

Just a suggestion, it would be great to be consistent with Poetry by default. So that people (my coworkers just did 😅) won't be surprised when not pass any parameters by default. Thanks! 😃

— Reply to this email directly, view it on GitHub https://github.com/bartdorlandt/ansible-role-poetry/issues/2, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHT3H6EGXJ4YMXDXVYMA3LXUQ7SBANCNFSM6AAAAAA3KZYMCE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

hongbo-miao commented 1 year ago

Hmm, I was thinking to support all null, true, false for virtualenvs.in-project. Well the current change also works at least for our case. Thanks Barty! 🚀