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

Consider merging in optimist_xl #115

Closed
nanobowers opened this issue Dec 20, 2020 · 6 comments
Closed

Consider merging in optimist_xl #115

nanobowers opened this issue Dec 20, 2020 · 6 comments
Assignees

Comments

@nanobowers
Copy link
Collaborator

Would kindly suggest to merge in the changes in the Optimist_XL gem, which is a feature enhancement fork. Also, it addresses #88 and #97, plus some closed-but-unresolved issues.

https://github.com/nanobowers/optimist_xl

@Fryguy
Copy link
Member

Fryguy commented Jan 4, 2021

@Fryguy
Copy link
Member

Fryguy commented Jan 4, 2021

This is awesome work @nanobowers! I am kind of swamped coming back from break, but hopefully I can eyeball this in the coming week.

@kbrock
Copy link
Member

kbrock commented Jan 6, 2021

Thank you for your patience and thanks for forking this to show us how many of your great ideas work

@Fryguy
Copy link
Member

Fryguy commented Apr 26, 2024

@nanobowers If you're up for it, can you make some PRs to get the core functionality of optimist XL into optimist? As you said in the other PR, not every feature in optimist XL is for everyone, but I do believe most features are for everyone. If you can make a PR per feature we can debate those independently, and I would presume most would make it after discussion.

@Fryguy
Copy link
Member

Fryguy commented Apr 26, 2024

Alternatively maybe we can resurrect (or redo) #116 . I can't recall what we stalled on...something about chronic gem it seems?

@nanobowers
Copy link
Collaborator Author

Alternatively maybe we can resurrect (or redo) #116 . I can't recall what we stalled on...something about chronic gem it seems?

@Fryguy IIRC I caused a breaking change due to chronic support. I have never been a fan of the hidden "if you happen to have this other gem installed you get completely different behavior" and pulled it into a separate file so one would have to require "optimist/chronic" to explicitly call out that chronic style behavior was desired. But then that violated the single-file rule which has existed since the beginning... I'll close this and #116 and piece-part out a bunch of PRs.

@nanobowers nanobowers closed this as not planned Won't fix, can't repro, duplicate, stale Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants