Skip to content

Fix typos discovered by codespell #12228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Sep 18, 2023
Merged

Fix typos discovered by codespell #12228

merged 4 commits into from
Sep 18, 2023

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Sep 17, 2023 •
edited
Loading

https://pypi.org/project/codespell

codespell  --count --skip="./build/shtool,./sapi/cli/mime_type_map.h,./ext/*,./Zend/*" -q=3 \
          --ignore-words-list=ake,blong,clen,fo,gord,mis,mye,nam,miliseconds,nd,sav,ser,siz,statics,ths,suppressable

cclauss
cclauss commented Sep 17, 2023
View reviewed changes
sapi/fpm/fpm/fpm_events.h
@@ -43,7 +43,7 @@ int fpm_event_init_main(void);
int fpm_event_set(struct fpm_event_s *ev, int fd, int flags, void (*callback)(struct fpm_event_s *, short, void *), void *arg);
int fpm_event_add(struct fpm_event_s *ev, unsigned long int timeout);
int fpm_event_del(struct fpm_event_s *ev);
int fpm_event_pre_init(char *machanism);
int fpm_event_pre_init(char *mechanism);
const char *fpm_event_mechanism_name(void);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful review please.

Copy link
Contributor

@thg2k thg2k Sep 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK, it was already fixed in 462da6e but then probably an erroneus conflict resolution during merge of e2a5428 in 819df03 turned it back to 'machanism', that's why the corresponding variable in the .c file is already correct.

@alexdowad
Copy link
Contributor

Hmm. Well, no objection for mbstring test data, I guess... although the files are provided by the Unicode Consortium and were copied directly into php-src as downloaded from their site.

If ever the Unicode Consortium updates those files and we import the updated files, the same misspellings will come back. So if you really want to fix them for good, it might be good to contribute the fixes upstream.

Anyways, no objection from me.

@youkidearitai
Copy link
Contributor

If ever the Unicode Consortium updates those files and we import the updated files, the same misspellings will come back. So if you really want to fix them for good, it might be good to contribute the fixes upstream.

If some by chance, these data files changes, fixes to each time.
I don't know where to report upstream, but shall I report it.

Is it here? https://corp.unicode.org/reporting/website.html

@nielsdos
Copy link
Member

If ever the Unicode Consortium updates those files and we import the updated files, the same misspellings will come back. So if you really want to fix them for good, it might be good to contribute the fixes upstream.

If some by chance, these data files changes, fixes to each time. I don't know where to report upstream, but shall I report it.

Is it here? https://corp.unicode.org/reporting/website.html

I believe that's for problems with the websites. I believe this should be used for the data files: https://corp.unicode.org/reporting/error.html

@youkidearitai
Copy link
Contributor

@nielsdos Thanks. I tried to send.
スクリーンショット 2023-09-17 22 08 10

@nielsdos
Copy link
Member

Perhaps a diff would've been easier for them, but I don't think it's a big deal if it's already sent.

@youkidearitai
Copy link
Contributor

Perhaps a diff would've been easier for them, but I don't think it's a big deal if it's already sent.

Ah, I'm lazy😣. I hope member of Unicode Consortium that look this page.

Girgias
Girgias reviewed Sep 17, 2023
View reviewed changes
@cclauss cclauss requested a review from Girgias September 18, 2023 03:30
@petk
Copy link
Member

petk commented Sep 18, 2023

Maybe this one can be added here also:

--- a/.gitattributes
+++ b/.gitattributes
@@ -25,5 +25,5 @@
 /Zend/zend_vm_handlers.h linguist-generated -diff
 /Zend/zend_vm_opcodes.[ch] linguist-generated -diff
 
-# The OSS fuzz files are bunary
+# The OSS fuzz files are binary
 /ext/date/tests/ossfuzz*.txt binary

@Girgias Girgias removed the request for review from bukka September 18, 2023 10:07
@Girgias Girgias merged commit 886bf82 into php:master Sep 18, 2023
@cclauss cclauss deleted the codespell branch September 18, 2023 10:30
@Ken-Whistler
Copy link

We did see this, but have decided not to touch the two archival mapping tables. Note those mapping tables are posted in an "OBSOLETE" directory for a reason -- they are basically just sources for historical research, and are not intended to be used for current products. JIS0212.TXT dates from 1994, and KSX1001.TXT from 1999. Later updates have only been to deal with the out-of-date terms of use and contact links, and have left the data (and the rest of the header content) as is, including typos. Please see the warning notice at the top of the ReadMe.txt:
https://www.unicode.org/Public/MAPPINGS/OBSOLETE/EASTASIA/ReadMe.txt

@youkidearitai
Copy link
Contributor

@Ken-Whistler Thank you very much for confirming this. I understand how to handle archived mapping tables. Thanks again.

thg2k
thg2k reviewed Sep 28, 2023
View reviewed changes
win32/build/mkdist.php
@@ -271,7 +271,7 @@ function extract_file_from_tarball($pkg, $filename, $dest_dir) /* {{{ */
}

/* include a snapshot identifier */
$branch = "HEAD"; // TODO - determine this from SVN branche name
$branch = "HEAD"; // TODO - determine this from GitHub branch name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe "git" instead of "GitHub"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$branch = "HEAD"; // TODO - determine this from GitHub branch name
$branch = "HEAD"; // TODO - determine this from git branch name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thg2k thg2k thg2k left review comments

@alexdowad alexdowad alexdowad approved these changes

@nielsdos nielsdos nielsdos approved these changes

@Girgias Girgias Girgias approved these changes

Assignees
No one assigned
Labels
Category: Build System Extension: mbstring Extension: reflection Extension: session SAPI: cli SAPI: fpm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cclauss @alexdowad @youkidearitai @nielsdos @petk @Ken-Whistler @thg2k @Girgias