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

Remotes/trunk #612

Closed
Closed

Conversation

kukackajiri
Copy link
Contributor

There are updates to fix errors found by Parfait static code analysis.

@zimmerle
Copy link
Contributor

zimmerle commented Mar 1, 2014

Hi @kukackajiri, thanks for you contribution. This kind of quality measurements are very important, we appreciate your effort.

I've made a partial merge of your patch as you can see here: 62a6f22

Some parts I did not merged yet and i have made some comments along the code. I believe that it is important to split some huge functions into small ones, so that, error handler will be more efficient and will be more clear to us whenever a return in the middle of the function can impact or not in the well functioning of the software.

I have made some changes in the copy_rules function as you can see here: 4281e07f806f5b78ed275e5a88f0a6609ea34293, it is almost the same thing that you have suggested, except for the fact that i have added documentation and other code cosmetics.

Thanks,
F.

zimmerle pushed a commit that referenced this pull request Mar 3, 2014
Continuation of kukackajiri's work to provide fixes for errors pointed by
Parfait. The function copy_rules had an integer as return code but it was not
filed proper neither checked by its callers. This commit just adds sanity
checks and documentation for the copy_rules function. Marking were placed
on the copy_rules callers, but the return code is not handled yet.
For kukackajiri's work, see merge request: #612
@zimmerle zimmerle closed this Mar 20, 2014
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

2 participants