<div dir="ltr"><span style="font-size:12.8px">I think it's 'ok' to try to tidy up the modules a bit, but I would put the expectation</span><div style="font-size:12.8px">that this is not a priority for reviewers.</div><div style="font-size:12.8px">And honestly, I don't think we should ever lint on tests this kind of styling.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">What I believe is something that you could change is pause a bit the refactoring,</div><div style="font-size:12.8px">the review queue has got somewhat flooded by these changes this last week.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Regards</div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-03-24 14:46 GMT+01:00 Yolanda Robla Mota <span dir="ltr"><<a href="mailto:yolanda.robla-mota@hpe.com" target="_blank">yolanda.robla-mota@hpe.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I did an effort to review that, and some of the patches are merged. Although it's low priority, in general these patches have improved readability, and in some of the cases, have grouped the parameters in a more logical way. I always tend to space the parameters myself, and keep them ordered in a logical way because they are easier to maintain later, so I agreed with that patch, even if that was not reflecting style guides. And I also appreciate the high effort that Andrey has put on it.<br>
<br>
However, I can see there is a cost to maintain the modules that way. I will not -1 for that, I found one case and i just suggested to keep the order, but I think it's not a reason for people to refactor a patch.<br>
<br>
That's my two cents.<br>
<br>
Best<br>
Yolanda<br>
<br>
El 24/03/16 a las 14:38, Jeremy Stanley escribió:<div class="HOEnZb"><div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 2016-03-24 13:42:58 +0300 (+0300), Andrey Nikitin wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
By this message I want to start a discussion about using of the<br>
Puppet style guide [0] in the 'openstack-infra/puppet-*' projects.<br>
As you can see, I've created a lot of change-requests to the<br>
repositories some days ago. I started this work, because I saw,<br>
that those manifests have different styles of the code, therefore<br>
I wanted to refactor and make much better them. For example, some<br>
of them have unsorted and unstructured lists of variables, some of<br>
them have no docstrings in the body with description of used<br>
variables and so on. I suppose, that we can unify those manifests<br>
by following the puppet style guide, but opinions are divided.<br>
</blockquote>
I don't think opinions are especially divided about rules mandated<br>
by the style guide, so much as other changes you were introducing<br>
not mandated by the style guide (alphabetizing class parameters,<br>
aligning = assignment operators, et cetera).<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
My point of view is following: if we implement the style guide on<br>
puppet manifests, we will have unified and structured manifests<br>
with documentation for all classes. We can use it without<br>
alphabetically sorting of variables, of course.<br>
<br>
So, my questions to you are:<br>
1. Should we follow the style guide or not?<br>
2. What the recommendation we could implement on Openstack Infra manifests?<br>
</blockquote>
We've already stated in the past that for any modules besides<br>
openstack_project (system-config), i.e. those we're publishing to<br>
Puppetforge, we would follow rules mandated by the Puppet Style<br>
Guide. I think things like making sure we declare required<br>
parameters before optional ones, use docstrings for clarity, and so<br>
on are fine. Just be aware that changes which refactor otherwise<br>
syntactically and logically correct code will be low priority for<br>
most of our reviewers and will likely have to be rebased many times<br>
if they touch a lot of lines in a given file.<br>
</blockquote>
<br>
-- <br></div></div><span class="HOEnZb"><font color="#888888">
Yolanda Robla Mota<br>
Cloud Automation and Distribution Engineer<br>
<a href="tel:%2B34%20605641639" value="+34605641639" target="_blank">+34 605641639</a><br>
<a href="mailto:yolanda.robla-mota@hpe.com" target="_blank">yolanda.robla-mota@hpe.com</a></font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
_______________________________________________<br>
OpenStack-Infra mailing list<br>
<a href="mailto:OpenStack-Infra@lists.openstack.org" target="_blank">OpenStack-Infra@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra</a><br>
</div></div></blockquote></div><br></div>