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

Issue #26 add missing crud to element tree #38

Merged
merged 3 commits into from Apr 13, 2019

Conversation

2 participants
@OnesQuared
Copy link
Contributor

commented Apr 13, 2019

Not 100% sure if these are the kind that are needed, mostly create was a bit of a gamble.

OnesQuared added some commits Apr 12, 2019

Issue #26
added create like a constructor for now
@ThomasJayWilliams

This comment has been minimized.

Copy link
Owner

commented Apr 13, 2019

Alright, I guess I'll check it later or either tomorrow. Today I have huge amount of work to do.

@ThomasJayWilliams ThomasJayWilliams self-assigned this Apr 13, 2019

@ThomasJayWilliams

This comment has been minimized.

Copy link
Owner

commented Apr 13, 2019

Alright, this is good enough. I'll provide some refactoring first, then I'll merge branch. For now there are no tasks for you and I would be able to add new ones today is for sure. I guess I'll add new issues for you tomorrow.

@OnesQuared

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2019

alright, no need to rush out to make issues, just put some out when you have the time. I'll be checking in time to time

@ThomasJayWilliams ThomasJayWilliams merged commit 7c9f208 into master Apr 13, 2019

@ThomasJayWilliams ThomasJayWilliams deleted the Issue-#26-Add-missing-CRUD-to-element-tree branch Apr 13, 2019

@ThomasJayWilliams

This comment has been minimized.

Copy link
Owner

commented Apr 13, 2019

Check out last commit. I've left some comments about changes.

@ThomasJayWilliams

This comment has been minimized.

Copy link
Owner Author

commented on 7c9f208 Apr 13, 2019

About Read - looks pretty weird: read that doesn't return anything.
About Delete - this won't work. List removes its elements by object reference. But you created another object with own reference, so method Remove will do nothing.
About Create - yeah, I forgot to tell - there are already Create methods provided - Add and AddRange.
About Update - it's a bad practice - replace list element. You should replace values of element, but not whole element.

@ThomasJayWilliams

This comment has been minimized.

Copy link
Owner

commented Apr 16, 2019

So, dude, I removed you from collaborators for now. I must be sure that project won't be changed even a bit for some time.
The current state of SDML.NET is "paused". I decided to develop standard for it first, and then, after it will be ready I will continue developing library. Thanks for help, you did a good job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.