Skip to content
This repository has been archived by the owner on Nov 19, 2019. It is now read-only.

Nest order #119

Closed
wants to merge 29 commits into from
Closed

Nest order #119

wants to merge 29 commits into from

Conversation

strazto
Copy link
Contributor

@strazto strazto commented Feb 4, 2019

Fixed nested group fold/unfold.
Added UI testing deps, logging dependencies, unit testing deps (IMO all necessary/ of use)

@yotamberk
Copy link
Owner

I sincerely appreciate this PR, but I don't want to introduce such a huge change in 1 PR.
I would really appreciate it if you could separate the Fix in one PR and the testing in another.

@strazto
Copy link
Contributor Author

strazto commented Feb 21, 2019

Thanks @yotamberk , will do.

@yotamberk
Copy link
Owner

@mstr3336 Any update?

@strazto
Copy link
Contributor Author

strazto commented Mar 9, 2019 via email

@yotamberk
Copy link
Owner

sorry, yotamberk, things got pretty busy in my life so the most i did for this is think about it occasionally and feel guilty.

No hard feelings! I myself am not too available for this project and truly understand the guilt feelings... Whenever you get around to this, it would be highly appreciated. I can implement your changes myself if you'd like, but I don't want to do it because I'd like you to have a commit in this project 😄

Just confirming, you're okay with logging statements if they're also in a separate PR?

Yup!

@strazto
Copy link
Contributor Author

strazto commented Mar 31, 2019

@yotamberk , my god this has taken me a long time to get to.
I saw someone describe a simpler fix on the issue I opened so I wonder if a fix based on that would make for a better PR.
I'm going to clean up my trees and put in one PR for logging, one for the unit tests, and I'll await your response before I work on the nested groups again.

@strazto
Copy link
Contributor Author

strazto commented Mar 31, 2019

Hey @yotamberk , see #149 and #150 for a much cleaner commit history + less busy PRs!

Let's pretend it didn't take me 2 months :'(

@strazto strazto closed this Mar 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants