search

f9micro / f9-kernel

An efficient and secure microkernel built for ARM Cortex-M cores, inspired by L4
Other
679 stars 145 forks source link

Added stm32l476 support, trivial kernel source code cleanup #148

Closed Kochise closed 7 years ago

Kochise commented 7 years ago

Cleaned things into two separate commits

jserv commented 7 years ago

@Kochise, Thanks again for sending pull request. Could you please use "git rebase -i` to rework these commits so that they can look cleaner and functioned?

Kochise commented 7 years ago

Thanks for the guidance, since I'm not a git expert.

Kochise commented 7 years ago

Done it, doesn't seems to have changed anything. More advices ?

louisom commented 7 years ago

Also, there is no need for rebase / sqashing commit, since when finish the PR, member can squash via GitHub UI, thanks!

Kochise commented 7 years ago

"for (int i = 0;" is considered bad coding practices, the declaration and affectation should be separated. All members and local variables should be placed at one location only to ease maintenance and avoid scope conflicts. Btw I separated these into two commits, the one regarding C99 styling is isolated, you just have to ditch it out if the changes really doesn't fits your taste.

Bear in mind that I developed my own 'personal' coding style, far beyond the 70s bearded nerd habits and/or the ioccc craze, which aims are to avoid all common pitfalls, ease debugging, maintenance and readability. If you dare to review my 'SkinProgress' code from 2003, I bet you'll call me a heretic. And my coding style even 'improved' since then, yet none of my public repos shows it actually.

I separated the 'ret =' because it is considered best practice and easier to debug to have a variable holding the value. You might want to use it in an 'assert' for instance. Clever 'optimizations' like this should be left to the compiler.

Copyright year is 2016 because the code changes were made last year, but only submitted this year. If that is really important, I will do so, just confirm.

About rebasing/sqashing, I have no much idea what it serves as a purpose and how to do it.

Thanks for your reviews and feedback.

Your truly.

edit : disambiguation

louisom commented 7 years ago

@Kochise friend, I think we both want this thing better, I must apologize for that I didn't catch up the previous PR discussion.

Your point about these variable make sense, and I think the copyright year probelm, if that is a new code (e.g. new board specific) need to move to 2017, others don't need to change.

Again, thanks for your helping and contribute. Louie.

Kochise commented 7 years ago

No problem, I'm rather happy to contribute to this nice project of yours :)

Will change copyright date to 2017 of the stm32l4 files. Will remove kernel code modifications to leave it as is.

Sorry for my rants about coding style. This is a very sensitive subject to me.