diffpy / diffpy.pdfgui

graphical user interface for real space structure refinement to PDF
Other
18 stars 27 forks source link

Errordialog #25

Closed dragonyanglong closed 6 years ago

dragonyanglong commented 6 years ago

This PR is for issue #24 . It includes the following edits in the following, which are committed one by one. I feel like the current commits looks clear. I can merge some commits into one or split one commit into multiple if needed. errorreportdialog.py is sync with design/errorreportdialog.wxg

update errorreportdialog with recent glade version drop error report form and send report button add Copy Error Log and Google This Error buttons and their event handler codes update the gui design, replacing all header text labels with one wx.html.HtmlWindow.

codecov[bot] commented 6 years ago

Codecov Report

Merging #25 into master will increase coverage by 0.04%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #25      +/-   ##
==========================================
+ Coverage   25.74%   25.79%   +0.04%     
==========================================
  Files          90       90              
  Lines       11916    11896      -20     
==========================================
  Hits         3068     3068              
+ Misses       8848     8828      -20
Impacted Files Coverage Δ
src/diffpy/pdfgui/gui/errorreportdialog.py 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ba9fb55...9674be3. Read the comment docs.

pavoljuhas commented 6 years ago

@dragonyanglong - I have finally got some time to check this PR and do a few fixups. Could you please check if this looks OK to you?

A few small comments regarding the commit style:

dragonyanglong commented 6 years ago

Hi @pavoljuhas , I looked through your commits, and they look great to me. Thanks for correcting and updating the codes for me.

I also rewrote my commit messages as you suggested. Are they look good to you? If not, please let me know, and I can change them accordingly. (I am not quite sure the difference btw MAINT and ENH) I used git rebase -i HEAD~10, and only changing my commit messages, but it seems like your last four commits are also updated on github website (although nothing changed in your commits, and git log looks all good).

Thanks a lot, Pavol!

dragonyanglong commented 6 years ago

BTW, the codecov/patch check looks failed, is that ok?

pavoljuhas commented 6 years ago

@dragonyanglong - thank you for checking and for improving commit messages. I actually meant that comment for future work, but if you already fixed the commits here it's great.

(I am not quite sure the difference btw MAINT and ENH)

To my understanding MAINT improves the existing code by fixing typos or refactoring (simplifying) the implementation (e.g., replacing hard-coded values with a module constant), but does not change what the affected functions do. ENH adds some new functionality or features that did not exist before.

BTW, the codecov/patch check looks failed, is that ok?

In this case yes. The unit tests in PDFgui cover only its control layer, but not the GUI elements.

dragonyanglong commented 6 years ago

I see. So maybe I should have used ENH for commit be5e879ce37b18b60aec467db3d1b8af9c701a73 because new features were added. Sorry about that. Anyway, I will keep in mind in the future. Thanks Pavol.

pavoljuhas commented 6 years ago

No worries it is not a big deal. Thank you for fixing the error dialog!