search

snu-quiqcl / qiwis

QuIqcl Widget Integration Software
MIT License
6 stars 2 forks source link

Becatrue/77/logging #81

Closed BECATRUE closed 1 year ago

BECATRUE commented 1 year ago

I implemented logging messages in numgen, poller, datacalc, dbmgr.

The detailed spec is writted in #77.

BECATRUE commented 1 year ago

image

Like the above logger, logging messages with updating database info are duplicated. I realized it is a problem in setDB(), so I will make a issue.

kangz12345 commented 1 year ago

A PR article should contain quite detailed explanation about "what" and "why" you implemented. If you explained something in issues, you should explicitly mention it in the main article of this PR so that the reviewers can focus on that points. Please mention related issues in the main article (or the initial comment of this PR). Moreover, you may use closing keywords to close the issues automatically when this PR is merged.

BECATRUE commented 1 year ago

A PR article should contain quite detailed explanation about "what" and "why" you implemented. If you explained something in issues, you should explicitly mention it in the main article of this PR so that the reviewers can focus on that points. Please mention related issues in the main article (or the initial comment of this PR). Moreover, you may use closing keywords to close the issues automatically when this PR is merged.

Okay! I'm sorry not to explain in details. But, I know if you used the closing keyword in commit messages, the issue will be closed automatically. Isn't it enough?

kangz12345 commented 1 year ago

But, I know if you used the closing keyword in commit messages, the issue will be closed automatically. Isn't it enough?

Oh, I wanted to talk about that. As you have experienced before, commit messages do not strongly belong to this PR. For example, if you do rebase or cherry-pick, commits can be transferred to other PRs. In this way, issues can be closed unexpectedly when the other PR is merged.

Therefore, I would like to suggest that we never write the closing keywords in commit messages, but in the PR main article. It also explicitly shows which issues will be closed. Note that people barely read the commit messages. Moreover, it does not appear on "Development" section of this PR (at the right of the page).

BECATRUE commented 1 year ago

I updated it!