cederom commented on PR #17105: URL: https://github.com/apache/nuttx/pull/17105#issuecomment-3362218278
> > > > @trns1997 thank you for the description updates, looks really good now :-) > > > > but we have CI failing :-( i have restarted just to make sure its not random problem.. https://github.com/apache/nuttx/actions/runs/18190378460/job/51796876198?pr=17105#step:6:2545 > > > > > > > > > ahhhhh, yes this will fail, there is sadly a cyclic dependencies between this PR and the one in the nuttx-app. Hmmm i could keep the guard active for this PR to pass and once its merged i'll create another PR removing it. What do you think? > > > > > > So this PR is supposed to be merged together with [apache/nuttx-apps#3186](https://github.com/apache/nuttx-apps/pull/3186) (which needs description update too) ? > > We should avoid this kind of changes, this is a breaking change because it will break compatibility between older/newer releases of either nuttx or nuttx-apps :-( > > There should be another way of approaching and solving the problem? Can you please try to find a way not to introduce additional dependency cycle between nuttx and nuttx-apps? If there is no other way then we should treat this as breaking change. > > Alright we'll split this into 2 PRs to get the pipelines happy. It is not really a breaking change fundamentally the apps will build it is just i removed a guard from the CI oot build test. It will just be considered as an unused variable but in the next PR we can remove it so the warning no longer shows Thanks @trns1997 :-) Its not just about warning but whether build is impacted anyhow for anyone around these commits. It should not be impacted at any step. If empty variable is the way then okay - nothing to built empty variable, something to built variable assigned, right? :-) You can update this PR so it builds fine without nuttx-apps part, then we can merge it safely, then we can merge nuttx-apps part, then we can merge next PR with the guard in nuttx repo correct? If so please provide 1 2 3 sequence to be applied. Would it be possible to do something similar with the PR in nuttx-apps or this is hard dependency on the nuttx part? -- 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]
