scylladb
add marshal test suite
Changes for serialization.Set was pushed, please check it for further work on the rest of the.
Please check it out, if everything is fine, I will squash the commits and continue working on test suite for the corrupt cases
not related to the code, but please squash commits, so they are sets of distinctive changes not "resolve conversation" ones
I unresolved two comments that I don't think were addressed, but maybe I am missing something there are a lot of changes
not related to the code, but please squash commits, so they are sets of distinctive changes not "resolve conversation" ones
I did it, divided by packages. There were a lot of changes and the code changed first one way, then the other, in such a situation it is very difficult to do something differently
I unresolved two comments that I don't think were addressed, but maybe I am missing something there are a lot of changes
Fixed
Thanks for addressing the comments! I would like you to fix the commit history a little bit, commits like "cleaning", "optimizing x", "simplify x" should not exist, squash them and make the first commits clean/optimal/simple etc. in the first place. Having such commits with fixes that could be incorporated into the original commits is not something we want to merge into our git history
Thanks for addressing the comments! I would like you to fix the commit history a little bit, commits like "cleaning", "optimizing x", "simplify x" should not exist, squash them and make the first commits clean/optimal/simple etc. in the first place. Having such commits with fixes that could be incorporated into the original commits is not something we want to merge into our git history
Something like this?
Right now, it's a single large commit. Ideally, it would be better to break it into smaller commits, but not in the way it was before, like "adding A" and then "optimizing part x of A." Instead, it should be more like "adding a" and then "adding b (not related to a)". For example, in this PR (https://github.com/scylladb/gocql/pull/256), each commit focuses on adding or fixing one specific thing. If a commit is fixing something, it's from the master branch or was added before that PR, not something introduced in the same PR. Does that make sense?
Right now, it's a single large commit. Ideally, it would be better to break it into smaller commits, but not in the way it was before, like "adding A" and then "optimizing part x of A." Instead, it should be more like "adding a" and then "adding b (not related to a)". For example, in this PR (#256), each commit focuses on adding or fixing one specific thing. If a commit is fixing something, it's from the master branch or was added before that PR, not something introduced in the same PR. Does that make sense?
In general, it makes sense.
But this PR does not change the current code at all, it is adding test suite.
This can be done in different ways, just tell me how.
resolve conversation v1...resolve conversation vn was needed so that the github would not lose conversations.
Right now, it's a single large commit. Ideally, it would be better to break it into smaller commits, but not in the way it was before, like "adding A" and then "optimizing part x of A." Instead, it should be more like "adding a" and then "adding b (not related to a)". For example, in this PR (#256), each commit focuses on adding or fixing one specific thing. If a commit is fixing something, it's from the master branch or was added before that PR, not something introduced in the same PR. Does that make sense?
In general, it makes sense. But this PR does not change the current code at all, it is adding
test suite. This can be done in different ways, just tell me how.
resolve conversation v1...resolve conversation vnwas needed so that thegithubwould not loseconversations.
I also agree that there is no reason to have it splitted into commits, even though it has 15 files, all of them a there to do exact one thing, which is in line with the commit message.
@illia-li , please don't resolve conversations, answer in the thread instead.
We will continue working on the
corrupt testshere?
No, let's merge it in and continue on another PR.