Fixes#969
Conversation
Underscores are used to decipher /dev/serial/by-id/ names.
Writing only the debug key seems not enough.
With recent changes, in probe we do no special handling for special targets. E.g. for STM32F7, DBGMCU is not yet changed in probe(). So after probe, target may again be in WFI. So use forced halt to regain control for attaching.
Write in ending independant way.
|
Could you please edit your PR to include a usable description of what issues these changes fix and their motivations so we can properly review it? |
|
Rachel, did you have a look at the commit messages in the 5 commits. They are mostly independant. The issues popped up when working on different subject. More explanation is in some of the text section of the commit messages. |
|
I did and I couldn't make heads not tails of the rationale to determine if they're good fixes or hiding deeper problems. I need context, context that is missing as the commit messages are too terse. You removed code, for example, marked as dealing with a nuance of a specific processor but haven't documented why so I can't tell if you broke something you couldn't test, or if it's harmless. |
|
@UweBonnes just to clarify. Don't assume that something that is obvious to you is obvious to others. You need to provide documentation, justification and context for the patches you submit. If you are trying to save your own time it will mean that I have to spend time thinking carefully about your patches before I merge them. Additionally. If there is more than one bug fix in here. Please split it out into their individual pull requests dealing with each issue separately so that we can discuss test and verify each patch on it's own. This will make for a cleaner and better process for everyone involved. Also please do not use generic "Fixes" topic for PR or Issues. Those don't mean anything. Consider choosing PR naming that explains the topic of the patch. I hope all this will result in a smoother and easier collaboration working on this project together. |
There was a problem hiding this comment.
Regarding the first patch in the series: It looks good to me. I am happy to merge that. In the description of the patch. I think you mean "slash" not "underscore". It would be good to correct the commit message wording to avoid confusion.
The other patches definitely need more context, and should probably be split out into their own pull requests. I am not confident if and what they are fixing.
In the last patch, the patch description talks about "ending independent way" do you mean "endianness independent" instead? To avoid confusion this wording should probably be fixed too.
3 participants
No description provided.