Re: [PR] fix misspelled names in locally scoped code [nuttx]
xiaoxiang781216 commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2876650026 @jerpelea please approve this patch. -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
xiaoxiang781216 commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2876648498 > > @sumpfralle not all spell error in the touch files fixed: > > yes, that's what I described before: I grouped my fixes into different categories in order to allow an easier review (separated into "trivial" / "easy" / "complicated" / "needs a decision"). > > Thus, I do not fix the spelling mistakes "by file", but "by category". This causes the checks to fail. > > For future _code contributors_ it is certainly fine and fair to request the fixing of spelling issues alongside code contributions. But that is a different situation. > > Thus, I would suggest to accept the imperfect state of the spelling checks for this PR. > > If accepting the current state is not suitable, then I would suggest to revert the "enable CI checks" commit, until my spelling fixing PRs are integrated. OK. -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
sumpfralle commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2870029622 > @sumpfralle not all spell error in the touch files fixed: yes, that's what I described before: I grouped my fixes into different categories in order to allow an easier review (separated into "trivial" / "easy" / "complicated" / "needs a decision"). Thus, I do not fix the spelling mistakes "by file", but "by category". This causes the checks to fail. For future *code contributors* it is certainly fine and fair to request the fixing of spelling issues alongside code contributions. But that is a different situation. Thus, I would suggest to accept the imperfect state of the spelling checks for this PR. If accepting the current state is not suitable, then I would suggest to revert the "enable CI checks" commit, until my spelling fixing PRs are integrated. -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
xiaoxiang781216 commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2869965904 @sumpfralle not all spell error in the touch files fixed: ``` ../nuttx/tools/checkpatch.sh -c -u -m -g b05ec7729d9b7fe85a6f11bfbb2da979255f44b7..HEAD Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/mx8mp/mx8mp_ecspi.c:335: occured ==> occurred Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/samv7/sam_1wire.c:693: re-use ==> reuse Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/stm32/stm32_1wire.c:831: re-use ==> reuse /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/stm32/stm32_1wire.c:948: ORE ==> OR Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/stm32l4/stm32l4_1wire.c:745: re-use ==> reuse Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:75: configed ==> configured /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:126: configed ==> configured /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:274: beacause ==> because /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:372: volatge ==> voltage /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:432: chanel ==> channel /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:540: diable ==> disable /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:831: paramters ==> parameters /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:978: resetted ==> reset /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:1017: configed ==> configured /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:1041: configed ==> configured /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:1150: configed ==> configured /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_adc.c:1152: configed ==> configured Used config files: 1: .codespellrc Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_pwm.c:463: intialize ==> initialize /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/tlsr82/tlsr82_pwm.c:702: Diable ==> Disable Used config files: 1: .codespellrc Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/arch/xtensa/src/esp32/esp32_himem_chardev.c:411: registerd ==> registered, registers Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/drivers/modem/alt1250/altmdm.c:86: pakcet ==> packet Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/drivers/usbhost/usbhost_bthci.c:466: dscriptors ==> descriptors Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/libs/libc/time/lib_localtime.c:349: offsetp ==> offset /home/runner/work/nuttx/nuttx/nuttx/libs/libc/time/lib_localtime.c:1277: offsetp ==> offset /home/runner/work/nuttx/nuttx/nuttx/libs/libc/time/lib_localtime.c:1291: offsetp ==> offset /home/runner/work/nuttx/nuttx/nuttx/libs/libc/time/lib_localtime.c:1299: offsetp ==> offset /home/runner/work/nuttx/nuttx/nuttx/libs/libc/time/lib_localtime.c:1299: offsetp ==> offset Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:76: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:77: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:78: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:82: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:87: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:93: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:94: Te ==> The, Be, We, To /home/runner/work/nuttx/nuttx/nuttx/libs/libdsp/lib_pmsm_model.c:101: Te ==> The, Be, We, To Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used config files: 1: .codespellrc Used con
Re: [PR] fix misspelled names in locally scoped code [nuttx]
sumpfralle commented on code in PR #16319: URL: https://github.com/apache/nuttx/pull/16319#discussion_r2083545595 ## libs/libc/wchar/lib_wcslcat.c: ## @@ -42,27 +42,27 @@ * Name: wcslcat * * Description: - * Appends src to string dst of size siz (unlike wcsncat, siz is the - * full size of dst, not space left). At most siz-1 characters - * will be copied. Always NUL terminates (unless siz == 0). - * Returns wcslen(initial dst) + wcslen(src); if retval >= siz, + * Appends src to string dst of size "size" (unlike wcsncat, "size" is the Review Comment: Understood and fixed now. Thanks for the pointer. -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
xiaoxiang781216 commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2869895349 > > yes, not all errors are fixed yet. All patch need pass ci check before get merging: [..] > > I would prefer, to postpone this. > > Locally I have another two or three PRs waiting, which will fix all remaining issues. It would burden the review process (and take some time for me), to re-bundle these in order to please the CI checks for this PR. > > The main remaining patchset contains this: > > > 1719 files changed, 3243 insertions(+), 3071 deletions(-) Not fix all spelling error in one patch or pr, but your change introduce new warning, which must be fixed, like this: https://github.com/apache/nuttx/pull/16319/files#r2083538730 -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
xiaoxiang781216 commented on code in PR #16319: URL: https://github.com/apache/nuttx/pull/16319#discussion_r2083538730 ## libs/libc/wchar/lib_wcslcat.c: ## @@ -42,27 +42,27 @@ * Name: wcslcat * * Description: - * Appends src to string dst of size siz (unlike wcsncat, siz is the - * full size of dst, not space left). At most siz-1 characters - * will be copied. Always NUL terminates (unless siz == 0). - * Returns wcslen(initial dst) + wcslen(src); if retval >= siz, + * Appends src to string dst of size "size" (unlike wcsncat, "size" is the Review Comment: fix the long line here. -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
sumpfralle commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2869830631 > yes, not all errors are fixed yet. All patch need pass ci check before get merging: [..] I would prefer, to postpone this. Locally I have another two or three PRs waiting, which will fix all remaining issues. It would burden the review process (and take some time for me), to re-bundle these in order to please the CI checks for this PR. The main remaining patchset contains this: > 1719 files changed, 3243 insertions(+), 3071 deletions(-) -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
xiaoxiang781216 commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2868348038 > I would like to wrap this PR up. Is there anything missing? yes, not all errors are fixed yet. All patch need pass ci check before get merging:  -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
sumpfralle commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2867509144 I would like to wrap this PR up. Is there anything missing? -- 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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
sumpfralle commented on code in PR #16319:
URL: https://github.com/apache/nuttx/pull/16319#discussion_r2079236505
##
libs/libc/stream/lib_blkoutstream.c:
##
@@ -153,15 +153,15 @@ static ssize_t blkoutstream_puts(FAR struct
lib_sostream_s *self,
if (offset > 0)
{
- size_t copyin = offset + remain > sectorsize ?
Review Comment:
In general I agree with your approach of keeping "traditional" names for
variables or functions, even if they conflict with spelling due to a shortage
of characters.
In the specific case of `copyin`, I refined your search suggestion a bit in
order to filter out all occurrences of "COPYING". This reduced the number of
occurrences in the linux kernel source from 2.7k [down to
27](https://github.com/search?q=repo%3Atorvalds%2Flinux+%2Fcopyin%5Cb%2F&type=code)
with only two occurrences in a header file (i.e. "public").
Thus, I would argue, that "copyin" is not in widespread use. I suggest to
"fix" the spelling here (as done in the PR).
I am aware, that the name "copyin" can be understood as a good choice based
on the [nuttx coding standard for variable
names](https://nuttx.apache.org/docs/latest/contributing/coding_style.html#parameters-and-local-variables)
("short" and "minimal use of underscore"). I do not feel entitled to judge
here.
--
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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
anchao commented on code in PR #16319:
URL: https://github.com/apache/nuttx/pull/16319#discussion_r2074367178
##
libs/libc/stream/lib_blkoutstream.c:
##
@@ -153,15 +153,15 @@ static ssize_t blkoutstream_puts(FAR struct
lib_sostream_s *self,
if (offset > 0)
{
- size_t copyin = offset + remain > sectorsize ?
Review Comment:
could we keep the name copyin? There are similar naming in other kernel:
https://github.com/search?q=repo%3Atorvalds%2Flinux+copyin&type=code&p=1
--
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]
Re: [PR] fix misspelled names in locally scoped code [nuttx]
nuttxpr commented on PR #16319: URL: https://github.com/apache/nuttx/pull/16319#issuecomment-2851507216 [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No. This PR does not fully meet the NuttX requirements. While the summary is acceptable, the impact and testing sections are insufficient. **Impact:** While the author *claims* there is no impact, they need to explicitly answer NO/YES for each question. For example: * Is new feature added? NO * Is existing feature changed? NO * Impact on user (will user need to adapt to change)? NO * ...and so on for all impact questions. **Testing:** "No testing was conducted" is unacceptable. Even for trivial changes like typos, a basic build test should be performed and the output logged. The author needs to demonstrate that the code still compiles and ideally run some basic functionality tests related to the changed code. Even if the change seems trivial, unexpected issues can arise. The PR description should include: * Build Host(s) information as required * Target(s) information as required * Build logs (even if they are identical before and after) In short, while the changes themselves might be minor, the PR needs to follow the required template completely to ensure consistent review and prevent regressions. -- 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]
[PR] fix misspelled names in locally scoped code [nuttx]
sumpfralle opened a new pull request, #16319: URL: https://github.com/apache/nuttx/pull/16319 ## Summary [Codespell](https://github.com/codespell-project/codespell/) discovered a few typos in variable or function names. These misspelled words are used in strictly local scopes. Renaming these should not cause any problems. ## Impact * Changing the name of a local variable within a function should not cause issues. * One python function name was changed. But this function was only used in self-contained script (not a library module). * One item name in an X macro was changed. But this X macro was used only in a module (not visible via the corresponding header file). ## Testing No testing was conducted. -- 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]
