mxcube / mxcubeqt

Qt Front-end of MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
14 stars 34 forks source link

Rhfogh refactor UI #448

Closed rhfogh closed 1 year ago

rhfogh commented 1 year ago

Replacement for PR #446, which was made to the wrong branch.

Refactored GPhL UI popups (using JsonParamsGui), to be ready for porting to Web version.

Also thoroughly linted and added in-line type annotations.

Since in-line type annotations are a departure from the Google doc strings we normally use, it would be great to see some feedback on the typing style before merging. The in-line annotations are more compact, but give less visual clarity in the function definitions, so we may well want to stick with doc string annotations after all (and I am very willing o change back).

Extensively debugged in mock mode, but because of the magnitude of the change even more testing might be required.

agruzinov commented 1 year ago

In terms of docstrings, Google -like ones are indeed more clear, but of course requires more time. On the other hand writing the full docstring on the some function that is not yet completely "moved in" into the code would be a bit of overkill. I would prefer to start with the short ones and move to the longer ones when the code is "well-cooked" i.e. removing the function or change of the logic is less probable. Maybe the automatic documentation generation will also be something to think of using those doc-strings.

agruzinov commented 1 year ago

No specific comments on my side. Merging.

rhfogh commented 1 year ago

Thanks!

On 30/08/2023 12:57, agruzinov wrote:

Merged #448 https://github.com/mxcube/mxcubeqt/pull/448 into develop.

— Reply to this email directly, view it on GitHub https://github.com/mxcube/mxcubeqt/pull/448#event-10232153306, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACK5QECCBRDLBUVATO4G6BDXX4TCXANCNFSM6AAAAAA4EFKLYQ. You are receiving this because you authored the thread.Message ID: @.***>