<div dir="ltr"><div dir="ltr">On Thu, Mar 21, 2019 at 8:35 PM Umesh Singla <<a href="mailto:umeshksingla@macports.org">umeshksingla@macports.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>a) We have seen a quick demo of this already. However, the major part I think is missing is the search. We can brainstorm over the details like search-as-you-type, adding new ports etc according to the timeline. Not sure how much browsing by the first letter helps.</div></div></div></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Search-as-you-type would be good, and can be further supported with: "New Ports", "Related Ports" on port detail, "Popular Ports"- overall and for each category.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>b) You cannot have a class Ports when it represents a Port: <a href="https://github.com/arjunsalyan/MacPorts-Demo-App/blob/master/ports/models.py#L6" target="_blank">https://github.com/arjunsalyan/MacPorts-Demo-App/blob/master/ports/models.py#L6</a>.<br></div></div></div></div></div></div></div></div></div></div></div></blockquote><div> </div><div>Yes, there are some major code improvements to be done. I will finish these shortly.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>2. build statistics:<br></div><div><br></div><div>a) In Time Elapsed of builds, it would be incorrect to show time taken for only one of the build stages. Example, in the case of <a href="https://build.macports.org/builders/ports-10.12_x86_64-builder/builds/87301" target="_blank">https://build.macports.org/builders/ports-10.12_x86_64-builder/builds/87301</a>, total-time-taken (12 min 59 secs) is right to be shown and not "6 mins 45 secs".</div></div></div></div></div></div></div></div></div></div></div></blockquote><div>Thanks for picking this out.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>b) There are some things which seem hard-coded to me. I see '10.14_x86_64', '10.13_x86_64' at multiple places - in port detail view, build to database view and jinja templates. It's time to define some constants config file now. For build statuses as well. With a new release of macOS, we do not want to have to change multiple files in code. In this project, it is important that a part which works, it is accurate and complete.<br></div></div></div></div></div></div></div></div></div></div></div></blockquote><div> </div><div>What I have planned is to have a separate table of builders with relations to the build history table. Any upcoming versions can then easily be added to the table. Since, I wasn't fetching many logs from</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>c) Also, as Mojca mentioned, errors like these: <a href="http://frozen-falls-98471.herokuapp.com/ports/database/" target="_blank">http://frozen-falls-98471.herokuapp.com/ports/database/</a> should not be exposed. What is it intended to do anyway?<br></div></div></div></div></div></div></div></div></div></div></div></blockquote><div> </div><div>Initially, I used this to parse build history into the database. But now I am using a separate script- just forgot to remove this. Sorry. As for errors we will be throwing custom 404 for doesnotexist exceptions</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div></div><div>3. installation statistics:<br></div></div></div></div></div></div></div></div></div></div></div></blockquote><div>Thank you, I will look into this.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>As Mojca said, I am not seeing any way to provide code review on Github when it's already merged. Since you have the base application ready, it's time to use PRs. I would also advise starting to follow at least some of the PEP8 style guide conventions, it's good to follow clean code practices from the beginning. You can either use coala lint or pylint before pushing the code, if familiar.</div></div></div></div></div></div></div></div></div></div></div></blockquote><div><br></div><div> I can submit PRs to the temporary repository Mojca mentioned about once it is available. We can then have a very fresh start. I will make the initial commit after improving the code based on the suggestions.</div><div><br></div><div>Thanks</div><div>Arjun</div></div></div>