disable ACE scaled alerts per radiation working group - #2
Conversation
|
do I need to do this action, or can you, once Tom is happy? It looks fine to me. The other change, the 1 hour wait on SCS 107 re-arm, is trickier On Thu, Jun 6, 2013 at 10:30 AM, bspitzbart notifications@github.comwrote:
*-Scott * Chandra X-Ray Center |
There was a problem hiding this comment.
This commented out line is different from the original, which is confusing.
In general, now that you are using a version control system that makes it easy to track not only the code changes but also provide detailed commentary on the how and why, it's better to not leave old lines of code as comments. The github-workflow way to do this is to put a summary of the radiation working group decision into the pull request discussion (complete with links to the twiki page), along with a short (could be one or two line) summary of what was done in the code to effect the change.
There was a problem hiding this comment.
@bspitzbart - it looks like you started adding an entirely different patch, which is the SCS107 lockout. It's really much better if you do one functional change per pull request, particularly since the original was simple and didn't really require testing (vs. this update which will need to be tested). Maybe including this update was accidental?
|
@bspitzbart - at this point I would suggest that you essentially start over by going back to master and making a new branch with a somewhat different name. The required code patch is really simple, just delete the two code blocks that currently test the scaled p5 and p6 rates. Take out the entire code blocks and leave only emptiness, but put in a good commit message, that might look something like: Note the very short (< 70 characters) commit title, followed by a blank line, followed by a detailed description. The detailed description isn't always required, but in this case would be quite useful. Following this format is useful for github because the messages show up nicely formatted. That's all that will be needed and this can go through. |
|
BTW, when you format the commit as above you should see the magic when you issue the pull request, which is that it is all filled in nicely. |
No description provided.