search

rubund / graywolf

Other
107 stars 36 forks source link

Safety port to C99 #13

Closed leviathanch closed 6 years ago

leviathanch commented 7 years ago

Hi I've ported most of the code to the C99 standard. This makes everything safer because implicit definitions and so on are now prohibited and the defines are clearer. Please test in another branch and pull to master as soon as all bugs have been eliminated. Cheers David

rubund commented 7 years ago

Hi David,

Thanks!

This is surely the biggest pull request (in number of commits) I have ever seen. I will go over it as soon as I find time for it.

Great effort. Cheers Ruben

rubund commented 6 years ago

Hi again David,

Thanks for the big effort you have put into this. It is great to see the bison / flex usage also.

However, the way it is now, it is basically impossible for me to review and merge it. There are basically too many changes to get an overview. Believe me, I have tried twice, spending several hours, and given up. So I will maybe pick a few things here and there from the pull request that I find useful, but it will not be merged.

I would need to have separate pull requests for every major change such as:

Please also use "rebase" to reorder the commits, so that every commit is there for a reason.

I am sorry. I know this may sound grumpy, but I simply do not have the time the way the pull request is now. So you either have to convince people to switch to your branch or you will have to separate it into smaller pull requests ( or do nothing, and your work will sadly remain unused ). :)

I am sorry again.

Best regards, Ruben

leviathanch commented 6 years ago

Hi Ruben

Thanks for the big effort you have put into this. It is great to see the bison / flex usage also.

However, the way it is now, it is basically impossible for me to review and merge it. There are basically too many changes to get an overview. Believe me, I have tried twice, spending several hours, and given up. So I will maybe pick a few things here and there from the pull request that I find useful, but it will not be merged.

I would need to have separate pull requests for every major change such as:

  • Port to C99 (function declarations etc.) . - then nothing else in this pull request - functionality should be 100% the same afterwards - Switch to using flex / bison
  • Code cleaning. - Binary produced must be the same afterwards
  • Remove old not-used functionality
  • Change CMake setup
  • etc..

Please also use "rebase" to reorder the commits, so that every commit is there for a reason.

I am sorry. I know this may sound grumpy, but I simply do not have the time the way the pull request is now. So you either have to convince people to switch to your branch or you will have to separate it into smaller pull requests ( or do nothing, and your work will sadly remain unused ). :)

I am sorry again. Totally getting it. I'm aware of the extend of my changes. Maybe we can just declare a new release under which we both cooperate on my new version? I mean the changes make it more memory safe, no system-calls from C to bash anymore and no regexp scripts being executed with system anymore either. It's much better this way and it's easier to find bugs with my version because there are no external forks anymore which confuse gdb.

Maybe we just both cooperate on a repo... lets say the official repo of LibreSilicon.com ? We could use https://github.com/foshardware and both push our changes there for cooperation.

What do you say?

Cheers David

rubund commented 6 years ago

Maybe we can just declare a new release under which we both cooperate on my new version?

I mean the changes make it more memory safe, no system-calls from C to bash anymore and no regexp scripts being executed with system anymore either. It's much better this way and it's easier to find bugs with my version because there are no external forks anymore which confuse gdb.

Maybe we just both cooperate on a repo... lets say the official repo of LibreSilicon.com ? We could use https://github.com/foshardware and both push our changes there for cooperation.

What do you say?

SInce I am anyway not working actively on the code base these days, I guess it is maybe not such a bad idea to setup a more experimental branch that we both can push to. However, I would appreciate if you could run an interactive rebase (rebase -i) and group together some of these more than 250 commits first! Especially when there are X number of commits with the same commit message after each other. It looks like you have used "git commit" as the save button in Word! :)

There are a number of other things which need to be sorted out in your branch also but they can be done after you have rebased it. (such as files with non-free licenses)

I will continue to keep my branch as a stable branch accepting bug fixes as they come. If the new experimental branch in the future becomes visibly more stable and feature rich I am sure most people (including me) will switch to that one.

Thank you

Best regards, Ruben

leviathanch commented 6 years ago

Hi

SInce I am anyway not working actively on the code base these days, I guess it is maybe not such a bad idea to setup a more experimental branch that we both can push to. However, I would appreciate if you could run an interactive rebase (rebase -i) and group together some of these more than 250 commits first! Ok! Will do this! ^^

Especially when there are X number of commits with the same commit message after each other. It looks like you have used "git commit" as the save button in Word! :) Haha! Caught me on that one! I was usually staging my stages in the Hackerspace for keeping on coding at home on the tower PC before running to the MTR ^_^'

There are a number of other things which need to be sorted out in your branch also but they can be done after you have rebased it. (such as files with non-free licenses) Ok

I will continue to keep my branch as a stable branch accepting bug fixes as they come. If the new experimental branch in the future becomes visibly more stable and feature rich I am sure most people (including me) will switch to that one. Ok!

Thanks for the feedback! I will do a rebase and lets talk further

Cheers David

rubund commented 6 years ago

Thanks for your interest in improving the project, and for the great effort you put into it.

Cheers Ruben

2018-01-26 9:52 GMT+01:00 David Lanzendörfer notifications@github.com:

Hi

SInce I am anyway not working actively on the code base these days, I guess it is maybe not such a bad idea to setup a more experimental branch that we both can push to. However, I would appreciate if you could run an interactive rebase (rebase -i) and group together some of these more than 250 commits first! Ok! Will do this! ^^

Especially when there are X number of commits with the same commit message after each other. It looks like you have used "git commit" as the save button in Word! :) Haha! Caught me on that one! I was usually staging my stages in the Hackerspace for keeping on coding at home on the tower PC before running to the MTR ^_^'

There are a number of other things which need to be sorted out in your branch also but they can be done after you have rebased it. (such as files with non-free licenses) Ok

I will continue to keep my branch as a stable branch accepting bug fixes as they come. If the new experimental branch in the future becomes visibly more stable and feature rich I am sure most people (including me) will switch to that one. Ok!

Thanks for the feedback! I will do a rebase and lets talk further

Cheers David

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/rubund/graywolf/pull/13#issuecomment-360720253, or mute the thread https://github.com/notifications/unsubscribe-auth/AB2VFs9GqNweQluPaHUEc0q7-1rYjBi8ks5tOZJOgaJpZM4Ouyiv .