[GitHub] [mynewt-core] kasjer commented on pull request #2858: hw/drivers/flash/spiflash: Add option to ignore JEDEC ID

[GitBox]

kasjer commented on PR #2858:
URL: https://github.com/apache/mynewt-core/pull/2858#issuecomment-1203517348

   @StarGate01 thanks for contributions


-- 
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: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [mynewt-core] kasjer commented on pull request #2858: hw/drivers/flash/spiflash: Add option to ignore JEDEC ID

[GitBox]

kasjer commented on PR #2858:
URL: https://github.com/apache/mynewt-core/pull/2858#issuecomment-1193666753

   @StarGate01 maybe you could change your commit a little bit to be more 
consistent with mynewt style that requires opening brace `{` to be on the same 
line as `if`.
   Since new syscfg values have default value 0 it can be safely used in plain 
`if` (not `#if`)
   Instead of having:
   ```c
   #if !MYNEWT_VAL(SPIFLASH_IGNORE_MANUFACTURER)
   assert(manufacturer == 
supported_chips[0].fc_jedec_id.ji_manufacturer);
   if (manufacturer != supported_chips[0].fc_jedec_id.ji_manufacturer) {
   rc = -1;
   goto err;
   }
   #endif
   ```
   we would have:
   ```c
   assert(!MYNEWT_VAL(SPIFLASH_IGNORE_MANUFACTURER) &&
  manufacturer == 
supported_chips[0].fc_jedec_id.ji_manufacturer);
   if (!MYNEWT_VAL(SPIFLASH_IGNORE_MANUFACTURER) &&
   manufacturer != supported_chips[0].fc_jedec_id.ji_manufacturer) {
   rc = -1;
   goto err;
   }
   ```
   generate code would be the same and compiler would check syntax that 
otherwise would not be checked in some build depending on ignore flags and 
mynewt style would be preserved without additional pre-processor conditions 
added for style sake only.
   
   You can amend your commit and force push.


-- 
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: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [mynewt-core] kasjer commented on pull request #2858: hw/drivers/flash/spiflash: Add option to ignore JEDEC ID

[GitBox]

kasjer commented on PR #2858:
URL: https://github.com/apache/mynewt-core/pull/2858#issuecomment-1191269201

   Hi, I am out of office, idea is good so definitely I will support it, I
   have not seen the code, I will take a look next week.
   
   Thanks for your submittion.
   Jerzy
   
   czw., 21 lip 2022, 10:17 użytkownik Christoph Honal <
   ***@***.***> napisał:
   
   > @kasjer  Is this something you would consider
   > merging? The feature would be opt-in.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > ,
   > or unsubscribe
   > 

   > .
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   


-- 
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: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org