-
Notifications
You must be signed in to change notification settings - Fork 137
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
Space the final frontier #1893
Space the final frontier #1893
Conversation
This is a HUGE change and therefore intended for post avrdude-8.0, I presume? IFF you want to enforce a certain coding style (and that is a VERY BIG IFF), could you please also add some tooling to consistently maintain that style for the future: Editor settings and formatting tool settings and git hooks for local use before pushing a commit, and CI checks which ensures mistakes in PRs are caught before merging them. Without that, the code inconsistency will inevitably increase again over time and the inconvenience this huge commit causes will have been wasted effort. Also, while the I still find confusingly inconsistent code formatting: I know my personal preferences (mostly Linux Kernel style, with exceptions e.g. for MISRA C style always { } after Exceptions to the consistent coding style should be made for source files which are copies of code from somewhere else, e.g. BTW: what tool did you use with which settings? That could be helpful for the tooling. Of course, while automatic code formatting tools do ensure consistency, they do not necessarily make things more readable. I personally would first verbally define a coding style and add that description to the git repo, then formally define it as configuration for one or several tools (tools should work on BSD, Linux, MacOS and Windows), and only then apply and enforce it. This PR only applies an undefined set of rules, some of which might be formally specified in Some discussion on coding style has happened on #1572, but I have not found that written down or specified formally anywhere in the |
Au contraire! This PR effects zero change in the sense below. It is safe to merge before v8.0. The created avrdude executable stays invariant except for the source line numbers that errors and warnings emit with
A hairy hand perhaps? 😉 Yes,
This has not completely been automated. I am sure there are better tools but, as they say, perfect is the enemy of good It is probably not possible to make
Correct. There is none, and I don't think the project has an aspiration of being prescriptive or trying to enforce style. What I found, however, is that contributors, say for a new programmer, tend to copy code for a similar programmer and then morph that into something that works. This perpetuates pain for maintenance. This PR will ensure that people start from a halfway reasonable style. So, all things considered, whilst I acknowledge that this PR might cause inconvenience, and that there will be deviations in the future, I still consider this PR a good investment into maintainability. And for the record: I am asking everyone who spent energy and thought on the style they used for their contribution to forgive this PR for riding roughshot over that.
Agreed, and I don't think this PR has touched those. Let me know specific examples where this PR has cast the net too wide. I plan to push a couple more commits onto this PR that are avrdude-specific and improve readability
Again, this will not change the executable. |
This PR reformats most of the C sources and removes tabs from some other files for consistent style.
There are some disadvantages of this, eg, diffs within git across this PR are going to be neigh impossible (though larger pieces of code changes in the recent past have already made diffs over a wide time span difficult). Weighing up improved maintenance (adding lines in files no longer poses minefields of working with sprinkles of tabs), enhanced readability through consistent code display, and modern debugging techniques (zooming in on commits that introduce bugs) that no longer require diffs over long time intervals has convinced me that this PR is beneficial.