search

ksh-community / ksh

ksh93 maintenance and development
Eclipse Public License 1.0
26 stars 11 forks source link

Fix 300+ typos found with the help of codespell #2

Closed McDutchie closed 4 years ago

McDutchie commented 4 years ago

This fixes typos in documentation, comments, and a few error messages. No code is changed.

jghub commented 4 years ago

some minor remarks:

  1. great! I believe typos in user-facing docs can send a message of "this is not thorough" and eliminating them is good/important.

  2. if theses fixes are merged it would be more to my personal taste to do it in human-sized bits not a massive 112 files changing single checkin. of course this also helps to verify thoroughly incrementally that all changes are desirable (and to bisect later if something was overlooked). so, say, 10 oder 100 checkins might be better than a single one (since I never was an active git(hub) user: maybe this affects the squash and merge thing up there?).

  3. the "user-invisible" typos in header and source files I would currently rather leave alone in order to be not distracted by spurious diffs during the upcoming patch-integration phase. but maybe others consider this a non-issue...

opinions?

McDutchie commented 4 years ago

@dannyweldon: I confirmed that ksh builds cleanly on Linux after this patch. (It doesn't build for me at all on macOS, which I usually use.)

jghub commented 4 years ago

@McDutchie the OSX build failure is known (I am hit by that one, too) and on the todo list. luckily OSX does provide its "own" /bin/ksh which also is 93u+ but apparently build from a somewhat older clone/download of the ATT/AST ksh-sources AFAICS.

McDutchie commented 4 years ago

@jghub : yeah, I'm well aware ksh ships with macOS, but I can't test my patches on that version :)

Re: "so, say, 10 oder 100 checkins might be better than a single one": I'm not going to spend hours splitting this work into 100-something commits. What I can do is split the comments typo fixes, the documentation typo fixes and the error message fixes, so 3 commits. And, if that is preferred for the coming patches integration phase, I can hold back the comments typo fixes.

McDutchie commented 4 years ago

Right, so this pull request now consists of two commits: one fixing 181 typos in user-facing documentation, and another fixing 47 typos in user-facing help and error messages.

Do you want me to push a third commit with the comments typo fixes as well, or leave that for now?

dannyweldon commented 4 years ago

opinions?

@jghub I don't care either way but it seems too much work to split up, no?

@McDutchie Sorry, my mistake, those :REQURES: typos are only in comments, so they're fine.

McDutchie commented 4 years ago

@jelmd - Would you care to elaborate on why you closed this?

jelmd commented 4 years ago

@McDutchie - closed by accident: I guess, github closed it automatically, when I renaming the master -> release. Strange is, that I did not get a notification about it (or it got buried somewhere else).

However, would you mind to create a new issue and then PR again?

Basically the issue itself should always be discussed on an issue page and the PR should refer to it by mentioning the #$issueNo. On the PR pages ideally code discussion should happen, only. Anyway, we currently putting such docs/guidelines together. So thanx for your patience!