-
Notifications
You must be signed in to change notification settings - Fork 184
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
v1.6.1 Feature List #128
Comments
Welcome to join me on branch |
I'm not sure to get it, looking at |
Ah I guess you squashed commits |
@leonardbinet Yes, I squashed and reviewed the patch last weekend. Your TODO: before we finish #127:
|
@caesar0301 well done for the implementation, I agree with you it is more elegant this way 👍 However why was it necessary for you to squash commits? If you want me to squash my commits in a single one to avoid having too many, no problem I understand 🙂. But the problem here is that you totally deleted my contribution from the history of the repository (as detailed here: isaacs/github#1303). This quite matters to me, since I see contributions to open-source projects are the main way for me to showcase my capabilities to potential recruitors etc. In this case I won't appear in git blame, history, or contributors of the repo, whereas I spend quite an effort trying to make a meaningful contribution. If so, you could simply repush |
@caesar0301 do you want to include serialization/deserialization #85 in this release? or should we release the current state and make another release in which we'll have time to discuss this separate subject? |
@leonardbinet It is fair to remain your efforts on the project through git logs. I compare both dev and dev_save branch and that is OK. |
@caesar0301 for the TODO:
-> I disagree, it is a fix, not an additional feature. If you don't set this
-> IMO it's not that bad but I might help on this, on which methods do you want to increase test coverage? |
In the library I plan to release I make use of this feature #138, would you agree to merge such feature in this release? |
It is reasonable to think about I agree with you that it is a fix. It is better to leave it for future release. |
It seems a helpful feature and not complicated to do. Let's release 161 when you finish this feature. |
v1.6.1 Feature List:
The text was updated successfully, but these errors were encountered: