<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 23, 2015 at 8:40 PM, Andrew Woodward <span dir="ltr"><<a href="mailto:xarses@gmail.com" target="_blank">xarses@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Denis,<div><br></div><div><span style="font-size:13.2px;line-height:19.8px">Now that I have better understanding of the history of the commit, I understand that this was the best way through. The Sahara and Murano team's effort was invaluable in getting these fixed up and in a good state. </span><span style="font-size:13.2px;line-height:1.5">I </span>apologize<span style="font-size:13.2px;line-height:1.5"> that I have raised this as an issue. I was very concerned with the commits before knowing theses details, It was necessary to get the clarification.</span></div><div><br></div><div>Let me clarify what I understand now was going on with them.</div><div><br></div><div>Sahara. </div><div>A )Fuel had a number of better parts of the fork. there where two commits [1][2] proposed to puppet-sahara from Fuel that where not merged that reflected the better side of Fuel's fork.</div><div>B) The Sahara sync commit [3] into fuel represented upstream puppet-sahara</div><div>C) The Adapt commit [4] contained the two commits listed prior in A, kilo support, stuff we needed to ensure it worked in fuel and Noop tests.</div><div><br></div><div>[1] <a href="https://review.openstack.org/#/c/198744/" target="_blank">https://review.openstack.org/#/c/198744/</a></div><div>[2] <a href="https://review.openstack.org/#/c/192721/" target="_blank">https://review.openstack.org/#/c/192721/</a></div><span class=""><div>[3] <a href="https://review.openstack.org/#/c/202045" target="_blank">https://review.openstack.org/#/c/202045</a></div><div>[4] <a href="https://review.openstack.org/#/c/202195/" target="_blank">https://review.openstack.org/#/c/202195/</a><br></div></span></div></blockquote><div><br></div><div>We will accept any patch that do not break backward compatibility for at least one release. <br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span class=""><div></div></span><div>Murano</div><div>D) Fuel has effectively the only usable Murano module</div><div>E) The Adapt commit [5] represented </div><div>* a major over hall of the code quality to make it suitable to propose upstream</div><div>* fixes necessary to support kilo</div><div>* cleanup for modular</div><div>* Noop tests</div></div></blockquote><div><br></div><div>If you consider to propose upstream, please follow this instructions to bootstrap the basic code structure: <a href="https://wiki.openstack.org/wiki/Puppet/New_module">https://wiki.openstack.org/wiki/Puppet/New_module<br></a><br></div><div>That will help you to have a compliant module from start.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>[5] <a href="https://review.openstack.org/#/c/203731/" target="_blank">https://review.openstack.org/#/c/203731/</a></div><div><br></div><div>With the improved clarity of what was going on, it made it much easier understand what I was reviewing and I'm glad of the current state.</div><div><br></div><div>Here are my thoughts on what we can do better next time:</div><div>* The commit and CR messages where not sufficient to understand entirely what was going on with the commits and how it was tested.</div><div>* Separate out some of the changes into a commit chain to reduce the scope of each CR so that its easier to review.</div><div>* For large reviews like this, we should let more reviewers know whats going on the ML early. These showed up on my radar late and of course, I freaked out.</div><div><br></div><div><br></div></div><div class=""><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 22, 2015 at 6:51 AM Denis Egorenko <<a href="mailto:degorenko@mirantis.com" target="_blank">degorenko@mirantis.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Andrew!<div><br></div><div>Sahara already merged. All CI tests were succeeded, also was built custom iso [1] and ran bvt tests [2], which also were succeeded and we got +1 from QA team.</div><div>For Murano we will do the same: resolve all comments, build custom iso, run custom bvt and wait +1 from Fuel CI and QA team.</div><div><br></div><div><p style="margin-top:0px;margin-bottom:0px;padding-top:0.5em;padding-bottom:0.5em;color:rgb(0,0,0);font-family:sans-serif">[1] <a href="http://jenkins-product.srt.mirantis.net:8080/view/custom_iso/job/custom_7.0_iso/562/" style="text-decoration:none;color:rgb(6,84,172)" target="_blank">http://jenkins-product.srt.mirantis.net:8080/view/custom_iso/job/custom_7.0_iso/562/</a></p><p style="margin-top:0px;margin-bottom:0px;padding-top:0.5em;padding-bottom:0.5em;color:rgb(0,0,0);font-family:sans-serif">[2] <a href="http://jenkins-product.srt.mirantis.net:8080/view/custom_iso/job/7.0.custom.ubuntu.bvt_2/131/" style="color:rgb(6,84,172)" target="_blank">http://jenkins-product.srt.mirantis.net:8080/view/custom_iso/job/7.0.custom.ubuntu.bvt_2/131/</a></p></div></div><div class="gmail_extra"><br><div class="gmail_quote"></div></div><div class="gmail_extra"><div class="gmail_quote">2015-07-22 0:41 GMT+03:00 Andrew Woodward <span dir="ltr"><<a href="mailto:xarses@gmail.com" target="_blank">xarses@gmail.com</a>></span>:<br></div></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-size:13.2px;line-height:19.8px">I was looped into reviewing the sync commits for Murano and Sahara. Both are in terrible shape and risk feature freeze at this point.</div><div style="font-size:13.2px;line-height:19.8px"><br></div><div><span style="font-size:13.2px;line-height:19.8px">We need feed back from the authors here. What is actually required for Kilo support (if any)from the Murano and Sahara modules? What will happen if these slip the release. What can you do to simplify the review scope. The most we can reasonably review is 500 LOC in any short time (and that's pushing it).</span></div><div style="font-size:13.2px;line-height:19.8px"><br></div><div style="font-size:13.2px;line-height:19.8px">Synopsis:</div><div style="font-size:13.2px;line-height:19.8px">murano [1] is -2, this can't be merged; there is a adapt commit with out any sync commit. The only way we will accept the fork method is a sync from upstream +adapt as documented in [2] also it's neigh impossible to review something this large with out the separation.</div><div style="font-size:13.2px;line-height:19.8px">-2 There is no upstream repo with content, so where did this even come from? We are/where the authority for murano at present so I'm baffled as to where this came from.</div><div style="font-size:13.2px;line-height:19.8px"><br></div><div style="font-size:13.2px;line-height:19.8px">Possible way through: A) Split sync from adapt, hopefully the adapt is small enough to to review. B)Make only changes necessary for kilo support.</div><div style="font-size:13.2px;line-height:19.8px"><br></div><div style="font-size:13.2px;line-height:19.8px">Sahara [3][4]</div><div style="font-size:13.2px;line-height:19.8px">This is a RED flah here, I'm not even sure to call it -1, -2 or something entirely else. I had with Serg M, This is a sync of upstream, plus the code on review from fuel that is not merged into puppet-sahara. I'm going to say that our fork is in much better shape at this moment, and we should just let it be. We shouldn't sync this until the upstream code is landed.</div><div style="font-size:13.2px;line-height:19.8px"><br></div><div style="font-size:13.2px;line-height:19.8px">Possible way through: C) The two outstanding commits inside the adapt commit need to be pulled out. They should be proposed right on top of the sync commit and should apply cleanly. I would prefer to see them as separate commits so they can be compared to the source more accurately. This should bring the adapt to something that could be reviewed. D) propose only the changes necessary to get kilo support.</div><div style="font-size:13.2px;line-height:19.8px"><br></div><div style="font-size:13.2px;line-height:19.8px">[1] <span style="font-size:13.2px;line-height:19.8px"><a href="https://review.openstack.org/#/c/203731/" target="_blank">https://review.openstack.org/#/c/203731/</a></span></div><div style="font-size:13.2px;line-height:19.8px">[2] <a href="https://wiki.openstack.org/wiki/Fuel/How_to_contribute#Adding_new_puppet_modules_to_fuel-library" target="_blank">https://wiki.openstack.org/wiki/Fuel/How_to_contribute#Adding_new_puppet_modules_to_fuel-library</a></div><div style="font-size:13.2px;line-height:19.8px">[3] <a href="https://review.openstack.org/#/c/202045" target="_blank">https://review.openstack.org/#/c/202045</a></div><div style="font-size:13.2px;line-height:19.8px">[4] <a href="https://review.openstack.org/#/c/202195/" target="_blank">https://review.openstack.org/#/c/202195/</a></div></div><span><font color="#888888"><div dir="ltr">-- <br></div><div dir="ltr"><p dir="ltr">--</p><p dir="ltr"><span style="font-size:13.2px">Andrew Woodward</span></p><p dir="ltr"><span style="font-size:13.2px">Mirantis</span></p><p dir="ltr"><span style="font-size:13.2px">Fuel Community Ambassador</span></p><p dir="ltr"><span style="font-size:13.2px">Ceph Community</span></p>
</div>
</font></span><br></blockquote></div></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr"><div><div dir="ltr"><div style="color:rgb(136,136,136)"><span style="font-family:arial;font-size:small">Best Regards,</span><br></div><span style="color:rgb(136,136,136)">Egorenko Denis</span>,</div><div><span style="color:rgb(136,136,136)">Deployment Engineer</span><br style="color:rgb(136,136,136)"><span style="color:rgb(136,136,136)">Mirantis</span><br></div></div></div></div>
</div>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div><div dir="ltr">-- <br></div><div dir="ltr"><p dir="ltr">--</p><p dir="ltr"><span style="font-size:13.2px">Andrew Woodward</span></p><p dir="ltr"><span style="font-size:13.2px">Mirantis</span></p><p dir="ltr"><span style="font-size:13.2px">Fuel Community Ambassador</span></p><p dir="ltr"><span style="font-size:13.2px">Ceph Community</span></p>
</div>
</div></div><br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr">Emilien Macchi<br></div></div>
</div></div>