maintainership conversation (and some of my fixes :P) #31
Conversation
Stuff like this is why i'm .... not feeling super confident about the microcontroller knowledge of previous devs.
this might be kinda academic, but i just couldn't stand this.
No effing comment.
|
I raised the same issue here: #16 I think your solution is brilliant. @stancecoke you can set branch protection rules on master and start giving access more freely to potential contributors who ask for it (this would allow them to push to all other branches but not master). Then indeed we'll be able to collaborate. |
|
I agree with your solution, I'd like to contribute too, but I don't know where. I've started with https://github.com/consp/BMSBattery_S_controllers_firmware/tree/master, which has some nice changes of its own too. But now I realise pullrequests over on another fork is not ideal. |
|
@stancecoke I think it'd really help if you added protection rules to the main branch but gave people commit rights for other branches more freely. Then this repo would be the main one for collaboration. Otherwise, even after a few years, we're still seeing more and more forks… |
|
BTW I reviewed the changes in this PR and everything makes sense. The commit descriptions are a bit angry but I like the energy. 😅 I don't feel good about jars in the repo but this is something that can be fixed later. Overall, good to go! ❤️ |
|
I'm not maintaining this project any longer. If someone wants to continue it, feel free. I can add a hint in the readme, which fork is the "Master" now. Or we can go back to @casainhos https://github.com/OpenSourceEBike/ repo. |
|
|
Not meant for merging since it's pretty much only meant for my bike. Could cherry-pick some parts of it if there is an active master and someone is willing to review the changes (e.g. the updated map functions, fixed point-ish math instead of floats, LCD8 stuff). The fork will also not likely compile on windows since I altered the makefiles to better work on linux (and switched to a later version of the compiler) and haven't tested on windows. I started out with MatGB's branch since it contained the LCD8 stuff but in the end I completely rewrote that part and merged stuff from other forks as well so that will definitely not merge as smooth as you like. Also redid the structure to be a bit more sane (split headers/source for instance) to be in line with most C programming. |
|
We can grant you write access to the Open E-Bike Organization, but we need someone who will maintain the project. |
I've changed windows building to work with the sane file structure which is way better. I made a pullrequest to your fork for it but understand that is not really the way to merge the overal project. Cherrypicking certain files of certain commits will be a lot of work but possibly worth it. Cherrypicking commits and merging different pullrequests might be the way, but is a very large and messy task. |
Ah , hadn't noticed it, for some reason github didn't show it (or I clicked it away sorry).
My idea was more to select the generally useful improvements and copy/paste them into a new branch, shouldn't be too hard but indeed a bit of work. |
Much easier, you don't care about receiving credit / maintaining commit history? |
final changes are "credit" and commit history ... mwa, mostly my mistakes so they can be erased. The orignal branch still exists for all it's relative glory. If I have to chose less work vs "credit" i'll chose less work, credit is generally meaningless. If someone needs it for their job sure go ahead but in my case that won't apply. |
There was a problem hiding this comment.
Good changes but had some comments. Would also like to see a change from a commit in a different branch of yours (473329e)[https://github.com/urjaman/BMSBattery_S_controllers_firmware/commit/473329e]
edit:
I might change these myself and post another pullrequest.
I have not been here a while, and I don't know how long I am going to stay. I'd like to be a maintainer in the sense of contributing, cleaning up codebase & bringing together forks(pullrequests) etc etc, but I do think the project should have multiple. This is the first small device stuff I have done with C so I still have things to learn(great opportunity).
| temph = *(uint8_t*) (0x53F2); | ||
| templ = READ_MMIO_U8(0x53F3); | ||
| temph = READ_MMIO_U8(0x53F2); | ||
| temph = READ_MMIO_U8(0x53F2); |
| @@ -21,3 +21,4 @@ Debug | |||
| /tools/JavaConfigurator/dist/ | |||
| /nbproject/private/ | |||
| /Result.log | |||
| experimental settings/*-*.ini | |||
There was a problem hiding this comment.
These might want to be shared nonetheless?
| @@ -48,6 +48,13 @@ uint8_t uart_get_buffered(void) { | |||
| } | |||
| return c; | |||
| } | |||
|
|
|||
| int uart_getch(void) { | |||
There was a problem hiding this comment.
Implementation incomplete / Is not used anywhere else? | Should be as safer way to get uart_get_buffered()?
| @@ -305,3 +319,30 @@ void watchdog_init(void) { | |||
| IWDG_SetReload(2); // 187.5us; for some reason, a value of 1 don't work, only 2 | |||
| IWDG_ReloadCounter(); | |||
| } | |||
|
|
|||
|
|
|||
| // Move these includes here if you want to see the identifiers that the ISR code uses | |||
Hi,
(This is a PR just to bring attention to the bits of generally usable
stuff i happened to do while making my bike run, the bigger things
i want to talk about follows...)
Reading the comments on an another pull request here,
there's nobody interested enough to be the singular maintainer for this.
I'm not signing up for that either, but i think this means we should adjust the
collaboration style.
My suggestion is: let's make a branch where everyone interested
gets direct push rights, so there's a place to integrate our work together,
otherwise there will only be endless forks without much
collaboration - mostly because we're not aware of the fixes other
people have made on their own forks...
My question is: where (which repo) should we place this?
(I'm asking because i'm the new guy here and i don't know what i dont know...)