acassis commented on PR #17573: URL: https://github.com/apache/nuttx/pull/17573#issuecomment-3689719548
> > Thank you @wangchdo amazing work and perfect reporting! :-) > > I have one issue, as there is a rename of very old and important file and functions inside (`sched/sched/sched_processtimer.c → sched/sched/sched_processtick.c`) this seems to be a breaking change, thus both PR and related git commit topic should contain `!` as the first character, plus description body should contain `BREAKING CHANGE: description on how API is changed plus quick fix instructions`. Maybe such invasive change is not really necessary? This will help existing users of that API to clearly notice a change and adapt their code. Thanks! :-) > > Hi @cederom, > > Thanks for your comments. > > I believe this change is not invasive. But to avoid any confusion, I have reverted the introduction of the new `nxsched_timer.c` file and kept using `sched_processtimer.c` without renaming it. > > Could you please help double-check the latest update of this PR? > > **Below is an explanation of why I consider this change to be non-invasive:** > > A new internal function, sched_process_tick(), has been added in sched/sched/sched_processtimer.c. All of the original logic previously implemented in sched_process_timer() has been moved into this new internal function, without any functional changes. > > sched_process_timer() has now been updated to act as a thin wrapper around sched_process_tick(). Its responsibility is limited to handling the differences in how OS ticks are driven—either directly by a hardware timer or via an hrtimer instance. > > The motivation for this refactoring is to clearly separate OS tick timer source selection from OS tick processing. The new internal function sched_process_tick() focuses solely on OS tick processing and preserves exactly the same behavior and logic as before. > > As a result: > > * When hrtimer is not enabled, sched_process_timer() behaves exactly as it did previously, simply delegating to sched_process_tick(). > > * When hrtimer is enabled, sched_process_timer() creates and uses an hrtimer instance to drive sched_process_tick(). > > > In summary, no existing behavior is changed. This refactoring only introduces a clearer separation of responsibilities while keeping all original functionality intact. The only addition is the internal sched_process_tick() function, which contains the same OS tick processing logic that was previously implemented directly in sched_process_timer(). > > @acassis, could you also help to double-check this update? @cederom just renaming an internal C file shouldn't be considered a breaking change if it didn't change the API. In other hand renaming a header file could be considered a breaking change, because now drivers needs to point to a new file. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
