base: update motorParker module #124
Conversation
guirodrigueslima
force-pushed
the
motorParker
branch
2 times, most recently
from
1789250 to
8a63179
Compare
guirodrigueslima
changed the title
guirodrigueslima
force-pushed
the
motorParker
branch
from
8a63179 to
d0ebb7c
Compare
|
On commit message: "The configure controller function" -> "The configureController function" |
|
I'm a bit annoyed by the entry in CHANGES.md. The v0.14.1 changelog shows an update in motorParker as a bug fix (#120). Now, we are adding the same entry as a new feature. Is there a way to disambiguate these two? |
|
The |
guirodrigueslima
force-pushed
the
motorParker
branch
2 times, most recently
from
3947a1b to
20d4123
Compare
CHANGES.md
Outdated
|
|
||
| * base: update motorParker module. by @guirodrigueslima in | ||
| https://github.com/cnpem/epics-in-docker/pull/124 | ||
| * motorParker module define motor resolution and motor status was included. |
There was a problem hiding this comment.
Message suggestion. Wdyt?
Added support to get motor status and to define motion resolution.
|
Commit message suggestion: base: update motorParker module.
|
guirodrigueslima
force-pushed
the
motorParker
branch
from
20d4123 to
3c2e688
Compare
henriquesimoes
left a comment
henriquesimoes
left a comment
There was a problem hiding this comment.
Could you please turn the commit message into non-itemized text paragraphs? That's the way we are used to have them in most Free and Open Source Software projects.
Here goes a suggestion:
base: update motorParker module.
A new feature has been implemented in the motorParker module to define
motor resolution. It is now possible to configure MRES and motor
resolution separately. In addition, the motor status MSTA field was
included. Furthermore, the configureController() function was created to
allow the controller to be reconfigured in case it is turned off and on
again.
Notice that I mostly added conjunctions, besides hard wrapping to 72 columns ;) In Vim, you should be able to wrap automatically by using gqip in normal mode.
|
Ops. I think our reviews crossed, @gustavosr8. |
guirodrigueslima
force-pushed
the
motorParker
branch
from
3c2e688 to
6581143
Compare
henriquesimoes
left a comment
henriquesimoes
left a comment
There was a problem hiding this comment.
@gustavosr8, please give a second look and see if any of your suggestions are still applicable, as @guirodrigueslima applied my suggestions.
Sorry about the overlapped review.
|
Just some modifications to simplify a little the commit message. Here is my suggestion:
|
No problem! Your suggestion was better than mine :) |
|
@guirodrigueslima, I'm not sure you noticed, but as soon as I merged #123 a conflict resolution became necessary for this PR. When you rebase it, please also take into account the Gustavo's last suggestion (which I tweaked slightly): |
guirodrigueslima
force-pushed
the
motorParker
branch
from
6581143 to
a9841f6
Compare
A new feature has been implemented to define motor resolution. It is now possible to configure it and MRES separately. In addition, the motor status MSTA field was included, and the configureController() function was created to allow the controller to be reconfigured in case it is restarted.
guirodrigueslima
force-pushed
the
motorParker
branch
from
a9841f6 to
219343d
Compare
|
|
||
| * base: update motorParker module. by @guirodrigueslima in | ||
| https://github.com/cnpem/epics-in-docker/pull/124 | ||
| * Support for motor resolution, motor status and reconfiguration upon reconnection added. |
There was a problem hiding this comment.
Just notice a small nit here. Please wrap the text at 79 chars
A new feature has been implemented in the motorParker module to define motor resolution. It is now possible to configure MRES and motor resolution separately.