Collect even more version numbers in docker/defaults/main.yml

samsapti commented

2022-12-28 15:24:38 +00:00

Owner

No description provided.

samsapti added 2 commits 2022-12-28 15:24:38 +00:00

Collect even more version numbers in docker/defaults/main.yml d6ce46e2f2

Make quotations consistent 231af48a40

samsapti added the

Refactor

label 2022-12-28 15:26:09 +00:00

samsapti added 1 commit 2022-12-28 15:46:59 +00:00

Make quotations consistent a10b07fa2c

valberg commented

2022-12-28 19:45:52 +00:00

Owner

Awesome! I guess you have tested this locally?

samsapti commented

2022-12-28 19:51:36 +00:00

Author

Owner

I guess you have tested this locally?

Actually not, but I have a similar setup here that works.

I can run it in Vagrant if you want me to.

> I guess you have tested this locally? Actually not, but I have a similar setup [here](/samsapti/pi-ansible/src/branch/main/roles/docker/defaults/main.yml) that works. I can run it in Vagrant if you want me to.

valberg commented

2022-12-28 20:02:37 +00:00

Owner

I guess you have tested this locally?

Actually not, but I have a similar setup here that works.

I can run it in Vagrant if you want me to.

Yes it would be nice, especially when touching so many files :)

> > I guess you have tested this locally? > > Actually not, but I have a similar setup [here](/samsapti/pi-ansible/src/branch/main/roles/docker/defaults/main.yml) that works. > > I can run it in Vagrant if you want me to. Yes it would be nice, especially when touching so many files :)

👍 1

samsapti commented

2022-12-28 20:38:48 +00:00

Author

Owner

Yes it would be nice, especially when touching so many files :)

PLAY RECAP *********************************************************************
default    : ok=109  changed=78   unreachable=0    failed=0    skipped=8    rescued=0    ignored=0

> Yes it would be nice, especially when touching so many files :) ``` PLAY RECAP ********************************************************************* default : ok=109 changed=78 unreachable=0 failed=0 skipped=8 rescued=0 ignored=0 ```

benjaoming commented

2022-12-29 11:01:59 +00:00

Owner

Nice that it works and with a focus on consistency!

Since we have ansible-lint, which also does yamllint, I got curious as to, why we would need any manual interventions on quotation marks... https://git.data.coop/data.coop/ansible/src/branch/main/.pre-commit-config.yaml

pre-commit run --all-files gives this result:

                             Rule Violation Summary                             
 count tag                           profile    rule associated tags            
     1 no-log-password                          opt-in, security, experimental  
                                                (warning)                       
     8 jinja[spacing]                basic      formatting                      
     8 name[missing]                 basic      idiom                           
     1 name[play]                    basic      idiom                           
     5 yaml[colons]                  basic      formatting, yaml                
     1 yaml[comments]                basic      formatting, yaml                
     3 yaml[empty-lines]             basic      formatting, yaml                
     6 yaml[indentation]             basic      formatting, yaml                
     6 yaml[line-length]             basic      formatting, yaml                
     3 yaml[new-line-at-end-of-file] basic      formatting, yaml                
     8 yaml[trailing-spaces]         basic      formatting, yaml                
     9 yaml[truthy]                  basic      formatting, yaml                
    67 name[casing]                  moderate   idiom                           
    31 risky-file-permissions        safety     unpredictability, experimental  
                                                (warning)                       
    58 fqcn[action-core]             production formatting                      
    37 fqcn[action]                  production formatting                      
     5 loop-var-prefix[missing]                 idiom                           
     9 loop-var-prefix[wrong]                   idiom                           

Failed after min profile: 234 failure(s), 32 warning(s) on 72 files.

I gather this - maybe I'm wrong: But looking at yamllinters and fixers, they don't actually care about quotation marks. I think it's because it's too complicated to lint. yamlfmt can force single quotes into double quotes, but I think that's it. I think - but not sure - it indicates that we hould not care about things that the linters don't care about, because they probably have their reasons :)

2.2 is a float, "2.2" is a string, but mostly it doesn't matter to write "2.2" anyways. Same for 2 (int) vs "2" (string). I think people who write a lot of yaml don't spend time on it and leave it to the linters.

We have some other interesting lint failures though :)

Nice that it works and with a focus on consistency! Since we have ansible-lint, which also does yamllint, I got curious as to, why we would need any manual interventions on quotation marks... https://git.data.coop/data.coop/ansible/src/branch/main/.pre-commit-config.yaml `pre-commit run --all-files` gives this result: ``` Rule Violation Summary count tag profile rule associated tags 1 no-log-password opt-in, security, experimental (warning) 8 jinja[spacing] basic formatting 8 name[missing] basic idiom 1 name[play] basic idiom 5 yaml[colons] basic formatting, yaml 1 yaml[comments] basic formatting, yaml 3 yaml[empty-lines] basic formatting, yaml 6 yaml[indentation] basic formatting, yaml 6 yaml[line-length] basic formatting, yaml 3 yaml[new-line-at-end-of-file] basic formatting, yaml 8 yaml[trailing-spaces] basic formatting, yaml 9 yaml[truthy] basic formatting, yaml 67 name[casing] moderate idiom 31 risky-file-permissions safety unpredictability, experimental (warning) 58 fqcn[action-core] production formatting 37 fqcn[action] production formatting 5 loop-var-prefix[missing] idiom 9 loop-var-prefix[wrong] idiom Failed after min profile: 234 failure(s), 32 warning(s) on 72 files. ``` I gather this - maybe I'm wrong: But looking at yamllinters and fixers, they don't actually care about quotation marks. I think it's because it's too complicated to lint. yamlfmt can force single quotes into double quotes, but I think that's it. I think - but not sure - it indicates that we hould not care about things that the linters don't care about, because they probably have their reasons :) 2.2 is a float, "2.2" is a string, but mostly it doesn't matter to write "2.2" anyways. Same for 2 (int) vs "2" (string). I think people who write a lot of yaml don't spend time on it and leave it to the linters. We have some other interesting lint failures though :)

samsapti commented

2022-12-29 15:42:37 +00:00

Author

Owner

@benjaoming the reason I've done this, is because the official YAML specification recommends so for readability's sake. Quotes should only be used in case special characters / escape sequences are used. Use double quotes if you need to escape characters (e.g. \n), single quotes if you have other characters not parsable if not quoted, otherwise no quotes at all.

From YAML spec:

The plain (unquoted) style has no identifying indicators and provides no form of escaping. It is therefore the most readable, most limited and most context sensitive style. In addition to a restricted character set, a plain scalar must not be empty or contain leading or trailing white space characters. It is only possible to break a long plain line where a space character is surrounded by non-spaces.

Plain scalars must not begin with most indicators, as this would cause ambiguity with other YAML constructs. However, the “:”, “?” and “-” indicators may be used as the first character if followed by a non-space “safe” character, as this causes no ambiguity.

As for the other linting errors and warnings, I think @valberg is working on that in #128.

@benjaoming the reason I've done this, is because the official YAML specification recommends so for readability's sake. Quotes should only be used in case special characters / escape sequences are used. Use double quotes if you need to escape characters (e.g. `\n`), single quotes if you have other characters not parsable if not quoted, otherwise no quotes at all. From [YAML spec](https://yaml.org/spec/1.2.2/#73-flow-scalar-styles): > The plain (unquoted) style has no identifying indicators and provides no form of escaping. It is therefore the most readable, most limited and most context sensitive style. In addition to a restricted character set, a plain scalar must not be empty or contain leading or trailing white space characters. It is only possible to break a long plain line where a space character is surrounded by non-spaces. > > Plain scalars must not begin with most indicators, as this would cause ambiguity with other YAML constructs. However, the “:”, “?” and “-” indicators may be used as the first character if followed by a non-space “safe” character, as this causes no ambiguity. As for the other linting errors and warnings, I think @valberg is working on that in #128.

benjaoming commented

2022-12-29 21:03:24 +00:00

Owner

Oh interesting! Then I was wrong! I was looking a lot at what the linters do but I couldn't come up with anything that would enforce/avoid manual labour in this area. I hope I'm wrong though because it seems like an annoying problem :/

samsapti commented

2022-12-29 21:06:55 +00:00

Author

Owner

Oh interesting! Then I was wrong! I was looking a lot at what the linters do but I couldn't come up with anything that would enforce/avoid manual labour in this area. I hope I'm wrong though because it seems like an annoying problem :/

Well, it's not a problem per se, it's more for readability. It doesn't hurt to use quotes, but avoiding them makes it more readable IMO.

> Oh interesting! Then I was wrong! I was looking a lot at what the linters do but I couldn't come up with anything that would enforce/avoid manual labour in this area. I hope I'm wrong though because it seems like an annoying problem :/ Well, it's not a *problem* per se, it's more for readability. It doesn't hurt to use quotes, but avoiding them makes it more readable IMO.

valberg commented

2022-12-31 23:34:20 +00:00

Owner

I have just kept to using quotes since there is the odd case of having to quote strings which start with a variable. But removing quotes doesn't rub me the wrong way :)

I have just kept to using quotes since there is the odd case of having to quote strings which _start_ with a variable. But removing quotes doesn't rub me the wrong way :)

samsapti commented

2023-01-02 09:43:34 +00:00

Author

Owner

I usually just quote everything that has a variable.

reynir commented

2023-01-02 11:24:26 +00:00

Owner

Removing the quotes changes the semantics in some cases. For example, services.nginx_acme_companion.version is now parsed as a float (2.2). I'd rather we quote everything over having to keep in mind how 2.2 is parsed as a float, no is parsed as a boolean etc.

Removing the quotes changes the semantics in some cases. For example, `services.nginx_acme_companion.version` is now parsed as a float (`2.2`). I'd rather we quote everything over having to keep in mind how `2.2` is parsed as a float, `no` is parsed as a boolean etc.

samsapti commented

2023-01-02 11:29:25 +00:00

Author

Owner

Removing the quotes changes the semantics in some cases. For example, services.nginx_acme_companion.version is now parsed as a float (2.2). I'd rather we quote everything over having to keep in mind how 2.2 is parsed as a float, no is parsed as a boolean etc.

It doesn't really matter, if they're just used as variables. It would matter if something like 2.2 was used by itself, instead of how it is right now with fx. repo/image:{{ some_var_that_is_2.2 }}.

That's just what the YAML spec recommends.

> Removing the quotes changes the semantics in some cases. For example, `services.nginx_acme_companion.version` is now parsed as a float (`2.2`). I'd rather we quote everything over having to keep in mind how `2.2` is parsed as a float, `no` is parsed as a boolean etc. It doesn't really matter, if they're just used as variables. It would matter if something like `2.2` was used by itself, instead of how it is right now with fx. `repo/image:{{ some_var_that_is_2.2 }}`. That's just what the YAML spec recommends.

samsapti commented

2023-01-02 11:35:26 +00:00

Author

Owner

Actually, maybe you're right. Though I don't think we should quote scalars that are obviously strings. I propose we follow this reference: https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html

samsapti added 1 commit 2023-01-07 17:21:35 +00:00

Merge branch 'main' into unify_more_configurations a246dbf497

samsapti commented

2023-01-14 16:20:48 +00:00

Author

Owner

@valberg @benjaoming I'm gonna revert the quotation changes for now, so we can focus the PR on the version numbers. We can always get back to it at another point.

👍 1

samsapti added 2 commits 2023-01-14 16:25:29 +00:00

Revert "Make quotations consistent" 2f1c1887ba