Skip to content
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.

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

Typos #3375

Merged
merged 21 commits into from
Mar 30, 2023
Merged

Typos #3375

merged 21 commits into from
Mar 30, 2023

Conversation

dilyanpalauzov
Copy link
Contributor

No description provided.

@miconda
Copy link
Member

miconda commented Feb 14, 2023

Are you expecting to be many more pull requests related to typos?

We should wait and collect all in one PR and squeeze per module, otherwise there are more and more chances of conflicts when we need to backport fixes to the code.

@dilyanpalauzov
Copy link
Contributor Author

The changes here are for modules, whose name starts with “a”. I do not expect more typos in modules, whose name starts with “a”.

@miconda
Copy link
Member

miconda commented Feb 14, 2023

What I mean is in the global context of your planned work to fix the typos, which is very appreciated, but it has to be coordinated and planned somehow, because the commit messages are not very explicit and repeat, then backports can be affected, making the release procedure more difficult.

For example, the acc module got several PRs lately related to typos:

901d9cac2f 2023-02-04 15:28:48 +0200 Дилян Палаузов acc: typos
06ffb667b7 2022-12-24 16:38:35 +0200 Дилян Палаузов acc: typo fuNction
55c77f0966 2022-12-12 19:27:44 +0200 Дилян Палаузов acc: typos a/an

This PR has another commit to acc module. There are other types of typos, but then we need to understand how your work is planned and how can impact the development of the code. It is really nice to have good spelling, but if the writing/maintenance of the C code becomes too complex, we need to coordinate better.

Maybe we just keep this PR open until you consider all the spell checking work finished completely, then rebase and merge.

Of course, others can comment or suggest alternative to deal with it.

@henningw
Copy link
Contributor

Good to bring this up, it might be indeed cause some issues for backports. We might consider not backporting this spelling fixes to the stable branches, just fixes for "real" bugs. I think we got some hundreds of these commits in the last weeks, which is impressive, but quite some work to backport.

@miconda
Copy link
Member

miconda commented Feb 15, 2023

Problems can also be when backporting fixes, because if a line was changed for a spell fix in variable/function name or in the comment and the code fix changes the same line or adjacent lines, then backporting fails because it does not matches the context in the file.

So sometimes backporting typos can help, but if it is a continuous work that results in hundred of commits it becomes a big overhead to track everything, what was in old or new code, what was backported or not, etc ...

@dilyanpalauzov
Copy link
Contributor Author

I have changes on my computer concerning typos, and I submit them in batches for some modules. I have also changes which change typos and documentation, these I will submit in separate PR per modules, once I am confident with the changes.

The changes were caught by using regex and whatsoever. At this place I wanted to write that I plan no further changes, but while writing the previous sentence I looked for catched and replaced it locally with caught.

Since the typos were not backported so far I assume that there is no practice of backporting typos. The concerns raised by having several commits with the identical title, in the context of backporting typos, are not traceable by me. When it comes to the amount of PRs, this information is not part of the git history, so my understanding is that it is irrelevant if fixes in three modules will be spread in one, two or three PRs.

I do not plan more changes, than I have currently. But if I find something while reading the manuals, I might or might not submit rewording.

@dilyanpalauzov
Copy link
Contributor Author

I have also changes which change typos and documentation,

which change/add/remove sentences in the documentation

@miconda miconda merged commit 0b986ae into kamailio:master Mar 30, 2023
@dilyanpalauzov dilyanpalauzov deleted the typos branch April 1, 2023 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants