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

fix issues in list.[ch] #7

Closed
wants to merge 1 commit into from
Closed

fix issues in list.[ch] #7

wants to merge 1 commit into from

Conversation

hexingb
Copy link
Contributor

@hexingb hexingb commented Jan 15, 2016

add some validation check of input parameters.
add a callback function to clean up ListValue when remove entries.

@fragglet
Copy link
Owner

Please fix failing test.

test-cpp.cpp:167:16: error: too few arguments to function ‘void list_free(ListEntry*, ListFreeFunc)’

@fragglet
Copy link
Owner

Looks okay in theory; can you please rewrite your commits to separate them into logically separate changes? ie. not just one commit named "fix issues"; one to add input validation, another to add the callback function. Please include meaningful descriptions; see here for suggestions:

http://chris.beams.io/posts/git-commit/#seven-rules

@hexingb
Copy link
Contributor Author

hexingb commented Jan 16, 2016

Sorry for this. git is not much familiar to me. I'll rewrite these again.
BTW, what do you mean "remove this"? remove what?

@fragglet
Copy link
Owner

No problem, I understand.

"Remove this" was a reference to the terminating NULL in list_to_array(). This isn't necessary and I'd rather not add it.

@hexingb
Copy link
Contributor Author

hexingb commented Jan 16, 2016

OK. I'll rewrite these again. Thank you for your patience.

@hexingb hexingb closed this Jan 16, 2016
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