I think it's 'ok' to try to tidy up the modules a bit, but I would put the expectation that this is not a priority for reviewers. And honestly, I don't think we should ever lint on tests this kind of styling.
What I believe is something that you could change is pause a bit the refactoring, the review queue has got somewhat flooded by these changes this last week. Regards 2016-03-24 14:46 GMT+01:00 Yolanda Robla Mota <[email protected]>: > 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. > > 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. > > That's my two cents. > > Best > Yolanda > > El 24/03/16 a las 14:38, Jeremy Stanley escribió: > > On 2016-03-24 13:42:58 +0300 (+0300), Andrey Nikitin wrote: >> >>> By this message I want to start a discussion about using of the >>> Puppet style guide [0] in the 'openstack-infra/puppet-*' projects. >>> As you can see, I've created a lot of change-requests to the >>> repositories some days ago. I started this work, because I saw, >>> that those manifests have different styles of the code, therefore >>> I wanted to refactor and make much better them. For example, some >>> of them have unsorted and unstructured lists of variables, some of >>> them have no docstrings in the body with description of used >>> variables and so on. I suppose, that we can unify those manifests >>> by following the puppet style guide, but opinions are divided. >>> >> I don't think opinions are especially divided about rules mandated >> by the style guide, so much as other changes you were introducing >> not mandated by the style guide (alphabetizing class parameters, >> aligning = assignment operators, et cetera). >> >> My point of view is following: if we implement the style guide on >>> puppet manifests, we will have unified and structured manifests >>> with documentation for all classes. We can use it without >>> alphabetically sorting of variables, of course. >>> >>> So, my questions to you are: >>> 1. Should we follow the style guide or not? >>> 2. What the recommendation we could implement on Openstack Infra >>> manifests? >>> >> We've already stated in the past that for any modules besides >> openstack_project (system-config), i.e. those we're publishing to >> Puppetforge, we would follow rules mandated by the Puppet Style >> Guide. I think things like making sure we declare required >> parameters before optional ones, use docstrings for clarity, and so >> on are fine. Just be aware that changes which refactor otherwise >> syntactically and logically correct code will be low priority for >> most of our reviewers and will likely have to be rebased many times >> if they touch a lot of lines in a given file. >> > > -- > Yolanda Robla Mota > Cloud Automation and Distribution Engineer > +34 605641639 > [email protected] > > > > _______________________________________________ > OpenStack-Infra mailing list > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra >
_______________________________________________ OpenStack-Infra mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
