You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/08/11 19:30:34 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
xiaoxiang781216 opened a new pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565
## Summary
since the variable arguments are error prone and seldom used. See the dissussion here: https://github.com/apache/incubator-nuttx/issues/740
## Impact
## Testing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-674094453
> @patacongo could you review the new change which keep wdparam_t as before.
I just skimmed through the changes. This looks much better to me. Thank you.
I will wait until all checks pass before doing a file review.
> It's better to merge apache/incubator-nuttx-apps#361 first, and then rerun this PR again if no more issue is found.
Okay, will do.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-673054213
> @papatience ok the argument type keep as wdparam_t now.
> It's better to merge [apache/incubator-nuttx-apps#361](https://github.com/apache/incubator-nuttx-apps/pull/361) first, and then rerun this PR again if no more issue is found.
@xiaoxiang781216 Did you mean @patacongo ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-672979529
> No one should consider merging this PR in its current state. I will break many existing architectures and cannot be permitted on master. Please refer to the comments in my Requested Changes.
Shall we light DRAFT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-673058379
> @xiaoxiang781216 Did you mean @patacongo ?
Paul Patience hasn't been involved in the NuttX project for several years.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-674097975
> > @patacongo could you review the new change which keep wdparam_t as before.
>
> I just skimmed through the changes. This looks much better to me. Thank you.
>
> I will wait until all checks pass before doing a file review.
>
> > It's better to merge [apache/incubator-nuttx-apps#361](https://github.com/apache/incubator-nuttx-apps/pull/361) first, and then rerun this PR again if no more issue is found.
>
> Okay, will do.
Well, I see no reason to merge pache/incubator-nuttx-apps#361 first. That is guaranteed be break the build. If I merge both at once, there is a chance that the build will survive. I think that is the best route to go.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo merged pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-673876728
@patacongo could you review the new change which keep wdparam_t as before.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-672958423
No one should consider merging this PR in its current state. I will break many existing architectures and cannot be permitted on master.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-673035354
@papatience ok the argument type keep as wdparam_t now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-674097473
Noting the documentation change for #1501
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-673058379
> @xiaoxiang781216 Did you mean @patacongo ?
Paul Patience hasn't been involved in the NuttX project for several years. Patacongo basically refers to the leg (pata) of a howler monkey (congo).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-672984996
wdparm_t is the perfect type in this case since it is has this property:
sizeof(wdparm_t) == MAX(sizeof(FAR void *), sizeof(uint32_t)
So it is always big enough. FAR void * often is not big enough; uint32_t is sometimes not big enough.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-673035354
@papatience ok the argument type keep as wdparam_t now.
It's better to merge https://github.com/apache/incubator-nuttx-apps/pull/361 first, and then rerun this PR again if no more issue is found.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1565: sched/wdog: Remove MAX_WDOGPARMS and related stuff
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1565:
URL: https://github.com/apache/incubator-nuttx/pull/1565#issuecomment-672958423
No one should consider merging this PR in its current state. I will break many existing architectures and cannot be permitted on master. Please refer to the comments in my Requested Changes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org