Re: [DISCUSS]: Self merge and Single company/organization merge gating
Hi, I agree that auto-merge should not be used. But I disagree that "as it is now since almost all patches follow the rule and seldom someone self-merges a patch". Here is a list of patches that were self merged last 12 days: https://github.com/apache/incubator-nuttx/pull/5474 https://github.com/apache/incubator-nuttx/pull/5445 https://github.com/apache/incubator-nuttx/pull/5444 https://github.com/apache/incubator-nuttx/pull/5428 https://github.com/apache/incubator-nuttx/pull/5425 https://github.com/apache/incubator-nuttx/pull/5508 All of the PRs have relatively low complexity and do not touch the core functionality so I'm ok with self-merging in such cases. Best regards, Petro пт, 18 лют. 2022 р. о 08:35 alin.jerpe...@sony.com пише: > > Hi all > > In my opinion we should not use the auto merge functionality since most of > the time there is at least 1 of us active at any time and the amount of > patches is not comparable to EX: Google. > > I think that the merge policy is fine as it is now since almost all patches > follow the rule and seldom someone self-merges a patch. > > Also we should note that in case some patches land accidentally in the master > branch we can always revert them if it is necessary > > Best regards > Alin > > -Original Message- > From: David Sidrane > Sent: den 17 februari 2022 22:31 > To: dev@nuttx.apache.org > Subject: RE: [DISCUSS]: Self merge and Single company/organization merge > gating > > On Self merge: > > As Nathan pointed out, it is more about time zones then merge velocity. > However, using a backport only methodology requires an upstream merge before > the work can be backported with least effort and adds a serial delay. It > would be ideal to reduces the CI quantum delay this as much as we can. > > GH has a setting to merge on successful CI after approval. It is lit by the > approver. This removes the polling for completion of CI. > If this can be configured it reduces the polling for both approver and > author. If it can not be configured in our repos, then self merge is the next > best thing. > > I am not trying to circumvent the review process at all - just remove the > idle time imposed by the process that is sampling related. > > > an approval from outside of the company/organization then the author > > can do the merge. For complex changes the person outside the > > organization should perform the merge even if there are more than 1 > > approval from inside the company/organization. > > I agree. > > David > > -Original Message- > From: Petro Karashchenko > Sent: Thursday, February 17, 2022 1:01 PM > To: dev@nuttx.apache.org > Subject: Re: [DISCUSS]: Self merge and Single company/organization merge > gating > > Hello, > > Regarding PRs megre by the author: I think that if the changes are relatively > simple (again that is very subjective, but I hope that people with merge > rights have more or less the same common sense of > it) and there is an approval from outside of the company/organization then > the author can do the merge. For complex changes the person outside the > organization should perform the merge even if there are more than 1 approval > from inside the company/organization. > > In this way reviewers can perform reviews with better quality and if someone > "forget" to press the "rebase & merge" button because for example CI is still > running and that is the end of working day, then the author can press that > button and not do extra tagging in PRs. I vote to make that process usable > for people and sacrifice bureaucracy in the places where it is possible. > > Best regards, > Petro > > вт, 15 лют. 2022 р. о 18:26 Nathan Hartman пише: > > > > On Mon, Feb 14, 2022 at 2:01 PM Brennan Ashton > > wrote: > > > > Background: > > > I am generally opposed to both of these. It is quite rare that we > > > need a crazy fast merge turn around on a PR. And if something is > > > approved and straight up broken in master that needs to get in then > > > I think forgiveness can be used to self merge. > > > > > > > > > I also generally do not have a big issue about people from the same > > > company reviewing and merging. I could see the arguments for shared > > > code but then I > > > think we are nitpicking. I prefer the velocity with a few oops that > > > can > > > be reverted along the way if needed. There is also parts of the > > > code base where the best people to review are on the same company. > > > > > > > > > I think most of the concerns here are best addressed not by process > > > but increasing the number of contributors who can participate. (more > > > committers and PPMC) > > > > Feel free to correct me if I'm mistaken, but I think David is bringing > > this up because of time zones. > > > > Indeed, most of the PR merging activity seems to occur during what I > > would call nighttime or early morning, and I think that might be more > > pronounced in David's time zone. > > > > Still, I think things have be
RE: [DISCUSS]: Self merge and Single company/organization merge gating
Hi all In my opinion we should not use the auto merge functionality since most of the time there is at least 1 of us active at any time and the amount of patches is not comparable to EX: Google. I think that the merge policy is fine as it is now since almost all patches follow the rule and seldom someone self-merges a patch. Also we should note that in case some patches land accidentally in the master branch we can always revert them if it is necessary Best regards Alin -Original Message- From: David Sidrane Sent: den 17 februari 2022 22:31 To: dev@nuttx.apache.org Subject: RE: [DISCUSS]: Self merge and Single company/organization merge gating On Self merge: As Nathan pointed out, it is more about time zones then merge velocity. However, using a backport only methodology requires an upstream merge before the work can be backported with least effort and adds a serial delay. It would be ideal to reduces the CI quantum delay this as much as we can. GH has a setting to merge on successful CI after approval. It is lit by the approver. This removes the polling for completion of CI. If this can be configured it reduces the polling for both approver and author. If it can not be configured in our repos, then self merge is the next best thing. I am not trying to circumvent the review process at all - just remove the idle time imposed by the process that is sampling related. > an approval from outside of the company/organization then the author > can do the merge. For complex changes the person outside the > organization should perform the merge even if there are more than 1 > approval from inside the company/organization. I agree. David -Original Message- From: Petro Karashchenko Sent: Thursday, February 17, 2022 1:01 PM To: dev@nuttx.apache.org Subject: Re: [DISCUSS]: Self merge and Single company/organization merge gating Hello, Regarding PRs megre by the author: I think that if the changes are relatively simple (again that is very subjective, but I hope that people with merge rights have more or less the same common sense of it) and there is an approval from outside of the company/organization then the author can do the merge. For complex changes the person outside the organization should perform the merge even if there are more than 1 approval from inside the company/organization. In this way reviewers can perform reviews with better quality and if someone "forget" to press the "rebase & merge" button because for example CI is still running and that is the end of working day, then the author can press that button and not do extra tagging in PRs. I vote to make that process usable for people and sacrifice bureaucracy in the places where it is possible. Best regards, Petro вт, 15 лют. 2022 р. о 18:26 Nathan Hartman пише: > > On Mon, Feb 14, 2022 at 2:01 PM Brennan Ashton > wrote: > > > Background: > > I am generally opposed to both of these. It is quite rare that we > > need a crazy fast merge turn around on a PR. And if something is > > approved and straight up broken in master that needs to get in then > > I think forgiveness can be used to self merge. > > > > > > I also generally do not have a big issue about people from the same > > company reviewing and merging. I could see the arguments for shared > > code but then I > > think we are nitpicking. I prefer the velocity with a few oops that > > can > > be reverted along the way if needed. There is also parts of the > > code base where the best people to review are on the same company. > > > > > > I think most of the concerns here are best addressed not by process > > but increasing the number of contributors who can participate. (more > > committers and PPMC) > > Feel free to correct me if I'm mistaken, but I think David is bringing > this up because of time zones. > > Indeed, most of the PR merging activity seems to occur during what I > would call nighttime or early morning, and I think that might be more > pronounced in David's time zone. > > Still, I think things have been working well, more or less, and I > don't think we need to make up any new rules right now. > > Instead, I would only urge committers to give complex PRs 12-24 hours > to percolate, even if there's an approving review, so other time zones > have a chance to look at them. > > Obviously that doesn't apply if it's urgent. For example, if the build > is broken and people can't get work done, or a serious error was > merged and needs to be reverted ASAP, don't wait, do it! > > Also, it's not necessary to delay for trivial PRs. > > What are the definitions of "complex," "trivial," "urgent," etc? I > say, committers should just use their best judgment and try to find a > good balance. Don't rush too much, don't delay too much. :-) > > David brings up a good point about time zones and we do have to > remember that NuttX is a global project, and I think that's the main > point to keep in mind. > > To Brennan's l
Re: RE: [DISCUSS]: Self merge and Single company/organization merge gating
Just my two cents, I like the self-merge policy. I am using it in all the repos where I can decide. My take on it is that it leaves the final responsibility for the change to the PR creator. And at the same time it doesn't take away the authority of the approvers. While the responsibility is left to the original PR creator, it actually adds one more decision point; the creator of the PR has one more chance to think again. I don't like the automatic merge on approval, I always want that the creator of the code presses the button, hence assuming responsibility. And as a + side, it can also speed up the merging. There is one thing that must be forced though; if the PR is changed after approval, a new approval must be required before self-merge can be done. Br, Jukka David Sidrane kirjoitti torstai 17. helmikuuta 2022: > On Self merge: > > As Nathan pointed out, it is more about time zones then merge velocity. > However, using a backport only methodology requires an upstream merge before > the work can be backported with least effort and adds a serial delay. It > would be ideal to reduces the CI quantum delay this as much as we can. > > GH has a setting to merge on successful CI after approval. It is lit by the > approver. This removes the polling for completion of CI. > If this can be configured it reduces the polling for both approver and > author. If it can not be configured in our repos, then self merge is the > next best thing. > > I am not trying to circumvent the review process at all - just remove the > idle time imposed by the process that is sampling related. > > > an approval from outside of the company/organization then the author can > > do the merge. For complex changes the person outside the organization > > should perform the merge even if there are more than 1 approval from > > inside the company/organization. > > I agree. > > David > > -Original Message- > From: Petro Karashchenko > Sent: Thursday, February 17, 2022 1:01 PM > To: dev@nuttx.apache.org > Subject: Re: [DISCUSS]: Self merge and Single company/organization merge > gating > > Hello, > > Regarding PRs megre by the author: I think that if the changes are > relatively simple (again that is very subjective, but I hope that people > with merge rights have more or less the same common sense of > it) and there is an approval from outside of the company/organization then > the author can do the merge. For complex changes the person outside the > organization should perform the merge even if there are more than 1 approval > from inside the company/organization. > > In this way reviewers can perform reviews with better quality and if someone > "forget" to press the "rebase & merge" button because for example CI is > still running and that is the end of working day, then the author can press > that button and not do extra tagging in PRs. I vote to make that process > usable for people and sacrifice bureaucracy in the places where it is > possible. > > Best regards, > Petro > > вт, 15 лют. 2022 р. о 18:26 Nathan Hartman пише: > > > > On Mon, Feb 14, 2022 at 2:01 PM Brennan Ashton > > wrote: > > > > Background: > > > I am generally opposed to both of these. It is quite rare that we > > > need a crazy fast merge turn around on a PR. And if something is > > > approved and straight up broken in master that needs to get in then > > > I think forgiveness can be used to self merge. > > > > > > > > > I also generally do not have a big issue about people from the same > > > company reviewing and merging. I could see the arguments for shared code > > > but then I > > > think we are nitpicking. I prefer the velocity with a few oops that > > > can > > > be reverted along the way if needed. There is also parts of the > > > code base where the best people to review are on the same company. > > > > > > > > > I think most of the concerns here are best addressed not by process > > > but increasing the number of contributors who can participate. (more > > > committers and PPMC) > > > > Feel free to correct me if I'm mistaken, but I think David is bringing > > this up because of time zones. > > > > Indeed, most of the PR merging activity seems to occur during what I > > would call nighttime or early morning, and I think that might be more > > pronounced in David's time zone. > > > > Still, I think things have been working well, more or less, and I > > don't think we need to make up any new rules right now. > > > > Instead, I would only urge committers to give complex PRs 12-24 hours > > to percolate, even if there's an approving review, so other time zones > > have a chance to look at them. > > > > Obviously that doesn't apply if it's urgent. For example, if the build > > is broken and people can't get work done, or a serious error was > > merged and needs to be reverted ASAP, don't wait, do it! > > > > Also, it's not necessary to delay for trivial PRs. > > > > What are the definitions of "complex," "trivial," "urgent," e
RE: [DISCUSS]: Self merge and Single company/organization merge gating
On Self merge: As Nathan pointed out, it is more about time zones then merge velocity. However, using a backport only methodology requires an upstream merge before the work can be backported with least effort and adds a serial delay. It would be ideal to reduces the CI quantum delay this as much as we can. GH has a setting to merge on successful CI after approval. It is lit by the approver. This removes the polling for completion of CI. If this can be configured it reduces the polling for both approver and author. If it can not be configured in our repos, then self merge is the next best thing. I am not trying to circumvent the review process at all - just remove the idle time imposed by the process that is sampling related. > an approval from outside of the company/organization then the author can > do the merge. For complex changes the person outside the organization > should perform the merge even if there are more than 1 approval from > inside the company/organization. I agree. David -Original Message- From: Petro Karashchenko Sent: Thursday, February 17, 2022 1:01 PM To: dev@nuttx.apache.org Subject: Re: [DISCUSS]: Self merge and Single company/organization merge gating Hello, Regarding PRs megre by the author: I think that if the changes are relatively simple (again that is very subjective, but I hope that people with merge rights have more or less the same common sense of it) and there is an approval from outside of the company/organization then the author can do the merge. For complex changes the person outside the organization should perform the merge even if there are more than 1 approval from inside the company/organization. In this way reviewers can perform reviews with better quality and if someone "forget" to press the "rebase & merge" button because for example CI is still running and that is the end of working day, then the author can press that button and not do extra tagging in PRs. I vote to make that process usable for people and sacrifice bureaucracy in the places where it is possible. Best regards, Petro вт, 15 лют. 2022 р. о 18:26 Nathan Hartman пише: > > On Mon, Feb 14, 2022 at 2:01 PM Brennan Ashton > wrote: > > > Background: > > I am generally opposed to both of these. It is quite rare that we > > need a crazy fast merge turn around on a PR. And if something is > > approved and straight up broken in master that needs to get in then > > I think forgiveness can be used to self merge. > > > > > > I also generally do not have a big issue about people from the same > > company reviewing and merging. I could see the arguments for shared code > > but then I > > think we are nitpicking. I prefer the velocity with a few oops that > > can > > be reverted along the way if needed. There is also parts of the > > code base where the best people to review are on the same company. > > > > > > I think most of the concerns here are best addressed not by process > > but increasing the number of contributors who can participate. (more > > committers and PPMC) > > Feel free to correct me if I'm mistaken, but I think David is bringing > this up because of time zones. > > Indeed, most of the PR merging activity seems to occur during what I > would call nighttime or early morning, and I think that might be more > pronounced in David's time zone. > > Still, I think things have been working well, more or less, and I > don't think we need to make up any new rules right now. > > Instead, I would only urge committers to give complex PRs 12-24 hours > to percolate, even if there's an approving review, so other time zones > have a chance to look at them. > > Obviously that doesn't apply if it's urgent. For example, if the build > is broken and people can't get work done, or a serious error was > merged and needs to be reverted ASAP, don't wait, do it! > > Also, it's not necessary to delay for trivial PRs. > > What are the definitions of "complex," "trivial," "urgent," etc? I > say, committers should just use their best judgment and try to find a > good balance. Don't rush too much, don't delay too much. :-) > > David brings up a good point about time zones and we do have to > remember that NuttX is a global project, and I think that's the main > point to keep in mind. > > To Brennan's last point: as we grow the committer base, we are likely > to have more people in more time zones and more PR reviewers, so this > should become less of a concern. > > Nathan
Re: [DISCUSS]: Self merge and Single company/organization merge gating
Hello, Regarding PRs megre by the author: I think that if the changes are relatively simple (again that is very subjective, but I hope that people with merge rights have more or less the same common sense of it) and there is an approval from outside of the company/organization then the author can do the merge. For complex changes the person outside the organization should perform the merge even if there are more than 1 approval from inside the company/organization. In this way reviewers can perform reviews with better quality and if someone "forget" to press the "rebase & merge" button because for example CI is still running and that is the end of working day, then the author can press that button and not do extra tagging in PRs. I vote to make that process usable for people and sacrifice bureaucracy in the places where it is possible. Best regards, Petro вт, 15 лют. 2022 р. о 18:26 Nathan Hartman пише: > > On Mon, Feb 14, 2022 at 2:01 PM Brennan Ashton > wrote: > > > Background: > > I am generally opposed to both of these. It is quite rare that we need a > > crazy fast merge turn around on a PR. And if something is approved and > > straight up broken in master that needs to get in then I think forgiveness > > can be used to self merge. > > > > > > I also generally do not have a big issue about people from the same company > > reviewing and merging. I could see the arguments for shared code but then I > > think we are nitpicking. I prefer the velocity with a few oops that can > > be reverted along the way if needed. There is also parts of the code base > > where the best people to review are on the same company. > > > > > > I think most of the concerns here are best addressed not by process but > > increasing the number of contributors who can participate. (more committers > > and PPMC) > > Feel free to correct me if I'm mistaken, but I think David is bringing > this up because of time zones. > > Indeed, most of the PR merging activity seems to occur during what I > would call nighttime or early morning, and I think that might be more > pronounced in David's time zone. > > Still, I think things have been working well, more or less, and I > don't think we need to make up any new rules right now. > > Instead, I would only urge committers to give complex PRs 12-24 hours > to percolate, even if there's an approving review, so other time zones > have a chance to look at them. > > Obviously that doesn't apply if it's urgent. For example, if the build > is broken and people can't get work done, or a serious error was > merged and needs to be reverted ASAP, don't wait, do it! > > Also, it's not necessary to delay for trivial PRs. > > What are the definitions of "complex," "trivial," "urgent," etc? I > say, committers should just use their best judgment and try to find a > good balance. Don't rush too much, don't delay too much. :-) > > David brings up a good point about time zones and we do have to > remember that NuttX is a global project, and I think that's the main > point to keep in mind. > > To Brennan's last point: as we grow the committer base, we are likely > to have more people in more time zones and more PR reviewers, so this > should become less of a concern. > > Nathan
Re: [DISCUSS] Default state of NDEBUG
Hello. My point about two config options was more because I think that apps and kernel are two separate entities and if there is a need to add extra debugging capabilities to the kernel it does not mean that it needs to be added for apps as well and vice versa. This seems to be right to me from the logical perspective especially if kernel mode build is selected. But going with a single option can be a solution as well. The other configuration options that you mention were mostly done to reach the tiny memory footprint rather than an attempt to violate POSIX. If we want to build a system that is capable of running from 8-bit to 64-bit embedded systems I think we can't afford to enable all the features by default. Best regards, Petro чт, 17 лют. 2022 р. о 10:52 Juha Niskanen (Haltian) пише: > > NDEBUG is just one macro name. I feel it is silly to have either one or two > CONFIG macros to control whether or not it is defined or not. My preference > is zero new CONFIG macros. We have too many already and trend seems to be > remove ones that disable standard POSIX features (CONFIG_DISABLE_SIGNALS, > CONFIG_DISABLE_POLL, CONFIG_CLOCK_MONOTONIC etc.) > > assert() and NDEBUG for apps should work exactly as in POSIX per Inviolables. > > If assert() is problem in kernel code, simply replace it with ASSERT() or > DEBUGASSERT(). > > Currently NuttX assert() macro expands into a C statement. This is incorrect, > as C standard requires it to expand to a void expression (a minor conformance > issue, most real programs won't notice the difference). > > -Juha > > From: Petro Karashchenko > Sent: Wednesday, February 16, 2022 10:31 PM > To: dev@nuttx.apache.org > Subject: Re: [DISCUSS] Default state of NDEBUG > > I'm not sure that I'm fully following the discussion (I will read the PR > comments to get the full context), but my vote is for: > 1. There should be a separate way to build kernel and app with assert() > enabled. > 2. The assert() should be disabled by default. So the default build is a > release build. > 3. I do not care if it is a config option or I need to "make NDEBUG=1" from > a command line. Probably 2 Kconfig options (one for kernel and 1 for apps > would be the best). > > Best regards, > Petro > > On Mon, Feb 14, 2022, 7:56 PM David Sidrane wrote: > > > PR 5399 adds an Kconfig option for NDEBUG. The salient discussion begins at > > [2] there are mixed positions and reasoning. xiaoxiang781216 asked me to > > raise a discussion on this. > > > > > > > > The reasoning for the Default state of to be NDEBUG (n) hence undefined so > > that assert() enabled is the following: > > > > > > > > 1) It follows the standard understanding of NDEBUG > > > > > > > > The standard for standard library from [3] > > > > > > > > The definition of the macro assert depends on another macro, > > NDEBUG, *which is not defined* by the standard library. > > > > > > > > > > > > 2) We have DEBUGASSERT for use in the OS. I believe this was an > > intentional separation on Greg’s part. We have asked for is input. > > > > > > > > In a NuttX "Release" build DEGUASSERT is off (all debug is off to show off > > the build size). > > > > I should still be able to build the app code with assert() and not have to > > use a Kconfig to enable it. > > > > How would you prefer it to be defined? > > > > > > > >1. Defaulted ON – assert() is a No OP > >2. Defaulted OFF assert() is enabled. > >3. Left to a command line setting from build system > > > > > > > > David > > > > > > > > [1] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-nuttx%2Fpull%2F5399&data=04%7C01%7Cjuha.niskanen%40haltian.com%7Cb69e018597b04f0f623608d9f18b7694%7C2f7c629267f24cc582be5d187e289ab2%7C1%7C0%7C637806403583675784%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=PfBujKU%2BW4LlfyvqZPmPy6rpUIbhWzTJpb93vs%2FbPic%3D&reserved=0 > > > > > > > > [2] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-nuttx%2Fpull%2F5399%23issuecomment-1029387606&data=04%7C01%7Cjuha.niskanen%40haltian.com%7Cb69e018597b04f0f623608d9f18b7694%7C2f7c629267f24cc582be5d187e289ab2%7C1%7C0%7C637806403583675784%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=a62%2FME0f8Iq1mMFqRdqqokm1EyuhVplv3r3RkQQjQTo%3D&reserved=0 > > > > [3] > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fc%2Ferror%2Fassert%23%3A~%3Atext%3DIf%2520NDEBUG%2520is%2520defined%2520as%2Cthe%2520source%2520code%2520where%2520%253Cassert.%26text%3DIf%2520NDEBUG%2520is%2520not%2520defined%2Coutput%2520and%2520calls%2520abort&data=04%7C01%7Cjuha.niskanen%40haltian.com%7Cb69e018597b04f0f623608d9f18b7694%7C2f7c629267f24cc582be5d187e289ab2%7C1%7C0%7C637806403583675784%7CUnknown%7CTWFpbGZsb3d8eyJWIj
Re: [DISCUSS] Default state of NDEBUG
NDEBUG is just one macro name. I feel it is silly to have either one or two CONFIG macros to control whether or not it is defined or not. My preference is zero new CONFIG macros. We have too many already and trend seems to be remove ones that disable standard POSIX features (CONFIG_DISABLE_SIGNALS, CONFIG_DISABLE_POLL, CONFIG_CLOCK_MONOTONIC etc.) assert() and NDEBUG for apps should work exactly as in POSIX per Inviolables. If assert() is problem in kernel code, simply replace it with ASSERT() or DEBUGASSERT(). Currently NuttX assert() macro expands into a C statement. This is incorrect, as C standard requires it to expand to a void expression (a minor conformance issue, most real programs won't notice the difference). -Juha From: Petro Karashchenko Sent: Wednesday, February 16, 2022 10:31 PM To: dev@nuttx.apache.org Subject: Re: [DISCUSS] Default state of NDEBUG I'm not sure that I'm fully following the discussion (I will read the PR comments to get the full context), but my vote is for: 1. There should be a separate way to build kernel and app with assert() enabled. 2. The assert() should be disabled by default. So the default build is a release build. 3. I do not care if it is a config option or I need to "make NDEBUG=1" from a command line. Probably 2 Kconfig options (one for kernel and 1 for apps would be the best). Best regards, Petro On Mon, Feb 14, 2022, 7:56 PM David Sidrane wrote: > PR 5399 adds an Kconfig option for NDEBUG. The salient discussion begins at > [2] there are mixed positions and reasoning. xiaoxiang781216 asked me to > raise a discussion on this. > > > > The reasoning for the Default state of to be NDEBUG (n) hence undefined so > that assert() enabled is the following: > > > > 1) It follows the standard understanding of NDEBUG > > > > The standard for standard library from [3] > > > > The definition of the macro assert depends on another macro, > NDEBUG, *which is not defined* by the standard library. > > > > > > 2) We have DEBUGASSERT for use in the OS. I believe this was an > intentional separation on Greg’s part. We have asked for is input. > > > > In a NuttX "Release" build DEGUASSERT is off (all debug is off to show off > the build size). > > I should still be able to build the app code with assert() and not have to > use a Kconfig to enable it. > > How would you prefer it to be defined? > > > >1. Defaulted ON – assert() is a No OP >2. Defaulted OFF assert() is enabled. >3. Left to a command line setting from build system > > > > David > > > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-nuttx%2Fpull%2F5399&data=04%7C01%7Cjuha.niskanen%40haltian.com%7Cb69e018597b04f0f623608d9f18b7694%7C2f7c629267f24cc582be5d187e289ab2%7C1%7C0%7C637806403583675784%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=PfBujKU%2BW4LlfyvqZPmPy6rpUIbhWzTJpb93vs%2FbPic%3D&reserved=0 > > > > [2] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-nuttx%2Fpull%2F5399%23issuecomment-1029387606&data=04%7C01%7Cjuha.niskanen%40haltian.com%7Cb69e018597b04f0f623608d9f18b7694%7C2f7c629267f24cc582be5d187e289ab2%7C1%7C0%7C637806403583675784%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=a62%2FME0f8Iq1mMFqRdqqokm1EyuhVplv3r3RkQQjQTo%3D&reserved=0 > > [3] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fc%2Ferror%2Fassert%23%3A~%3Atext%3DIf%2520NDEBUG%2520is%2520defined%2520as%2Cthe%2520source%2520code%2520where%2520%253Cassert.%26text%3DIf%2520NDEBUG%2520is%2520not%2520defined%2Coutput%2520and%2520calls%2520abort&data=04%7C01%7Cjuha.niskanen%40haltian.com%7Cb69e018597b04f0f623608d9f18b7694%7C2f7c629267f24cc582be5d187e289ab2%7C1%7C0%7C637806403583675784%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=kpoMBy%2BnLyiptB2GHnNBIS9Ln%2FEv7tn64tegKeV%2B3Nw%3D&reserved=0() > . >
