<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body dir="auto">
<br>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
Hi Steve:</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
Thanks for your great efforts for WSME replacement.</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
I had a quick review for your code changes, see my comments below:</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<table width="" cellspacing="0" cellpadding="1" style="border-collapse: collapse;">
<tbody>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Review<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Feedback<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">Reason<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">704490<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Has concern<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">move wsme code to ironic, types.py<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">704487<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Has concern<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171); width: 715px;">move wsme code to ironic, node.py<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">704489<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">has concern<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">move wsme code to ironic, args.py<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">704486<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">has concern <br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">move wsme code to ironic expose.py<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">703898<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Both positive and concern <br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">Positive, better code structure than original code, concern, more difficult to locate WSME code<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">704488<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">has concern<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">move wsme code to ironic<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">704485<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">has concern<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">Add pecan code, pecan would be replaced by flask<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">703897<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Positive<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">Removed some WSME code with this change<br>
</td>
</tr>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">703723<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Positive<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">Removed some WSME code with this change<br>
</td>
</tr>
</tbody>
</table>
<table width="" style="border-collapse: collapse;">
<tbody>
<tr style="background-color: rgb(255, 255, 255);">
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">703695<br>
</td>
<td style="width: 120px; border: 1px solid rgb(171, 171, 171);">Positive<br>
</td>
<td style="border: 1px solid rgb(171, 171, 171);">Removed some WSME code with this change<br>
</td>
</tr>
</tbody>
</table>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
Firstly, I appreciated 3 changes which definately removed some wsme code, especially the change 703897.</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
But I have some concerns for changes 704490, 704487, 704489, ..., these changes seemed move WSME code into ironic, that would make ironic code base become a bit large, from my personal view, to use python built-in feature or other 3-party lib to replace WSME
function would be better. I like the way that code change 703897 did. <br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
Thanks</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
Jerry</div>
<div style="font-size: 12pt; -webkit-text-size-adjust: auto; font-family: Calibri, Helvetica, sans-serif;">
<br>
</div>
<div dir="ltr">发自我的iPhone</div>
<div dir="ltr"><br>
<blockquote type="cite">在 2020年1月30日，上午8:53，Steve Baker <sbaker@redhat.com> 写道：<br>
<br>
</blockquote>
</div>
<blockquote type="cite">
<div dir="ltr"><span>I've put together a set of changes for removing WSME which involves copying just enough of it into ironic, and I think we need to have the conversation about whether this approach is desirable :) This git branch[1] finishes with WSME removed
and existing tests passing.</span><br>
<span></span><br>
<span>Here are some stats about lines of code (not including unit tests, calculated with cloc):</span><br>
<span></span><br>
<span>4500 wsme/wsme</span><br>
<span></span><br>
<span>6000 ironic/ironic/api master</span><br>
<span></span><br>
<span>7000 ironic/ironic/api story/1651346</span><br>
<span></span><br>
<span>In words, we need 1000 out of 4500 lines of WSME source in order to support 6000 lines of ironic specific API code.</span><br>
<span></span><br>
<span>Switching to a replacement for WSME would likely touch a good proportion of that 6000 lines of ironic specific API code. If we eventually replace it with a new library I think it would be easier if the thing being replaced is inside the ironic source
tree to allow for a gradual transition. So this approach could be an end in itself, or it could be a step towards the final goal.</span><br>
<span></span><br>
<span>My strategy for copying in code was to pull in chunks of required logic while removing some unused features (like request pass-through and date & time type serialization). I also replaced py2/3 patterns with pure py3. There is likely further things which
can be removed or refactored for simplicity, but what exists currently works.</span><br>
<span></span><br>
<span>If there is enough positive feedback for this approach I'll start on unit test coverage for the new code.</span><br>
<span></span><br>
<span>[1] https://review.opendev.org/#/q/topic:story/1651346</span><br>
<span></span><br>
<span></span><br>
</div>
</blockquote>
</body>
</html>