memoizable icon indicating copy to clipboard operation
memoizable copied to clipboard

Add devtools as a development dependency

Open dkubb opened this issue 11 years ago • 5 comments

This branch will not be merged into master directly. It's main purpose will be to setup devtools to run against memoizable code and specs to make sure it meets the same standards as other ROM related code.

Some commits from this branch may be cherry-picked into master, such as documentation, specs or code refactoring, but devtools specific code will not be merged into master until 1.8 compatibility is removed.

dkubb avatar Dec 02 '13 01:12 dkubb

@dkubb I think you can merge this one now, as we have a "18support in branch" strategy.

@sferik Would have to nuke this change again, which would be pretty easy IMHO.

Having a master with devtools across all rom related repos would help a lot.

mbj avatar Dec 16 '13 15:12 mbj

I thought we said we were going to wait until 1.0.0 was released to merge these changes into master? Pre-1.0.0 versions should retain Ruby 1.8.7 compatibility. It’s not clear (to me, at least) that this library is ready for a version 1.0.0 release. There is still significant churn, open questions about functionality/scope, and some crazy bugs.

sferik avatar Dec 18 '13 00:12 sferik

I'm not going to merge this into master yet. We need a period of non-volatility before we start talking about 1.0.

@sferik I wouldn't exactly classify that as a crazy bug in memoizable. It'd be different if it was documented as supporting marshal, but it doesn't; lack of support doesn't constitute a bug.

However, I am willing to look at adding support for marshal. I can probably eliminate the use of a Proc by having a named, callable singleton object that performs the same work. This should allow support with marshal, but of course we should add some integration tests to ensure it works.

I'll try to get to this within the next week or so, if anyone wants it sooner PRs are welcomed.

dkubb avatar Dec 18 '13 15:12 dkubb

I wouldn't exactly classify that as a crazy bug in memoizable. It'd be different if it was documented as supporting marshal, but it doesn't; lack of support doesn't constitute a bug.

Fair enough. I suppose there’s a line between a bug in a library and a bug caused by a library. I didn’t mean to imply it was the former.

That said, I hope you can appreciate my surprise to discover that a memozing a method made an object unmarshalable. I do hope this is something that is officially supported and tested in the future. As such, I have created a new issue for this feature.

sferik avatar Dec 18 '13 15:12 sferik

Yeah, no worries. Those kinds of issues will always be found as you introduce new constraints into the system.

For example, someone could use the library with JSON at some point in the future, or some other gem that depends on a specific hook method. I don't mind supporting them, within reason, as long as the complexity of the gem doesn't increase too greatly. I'm also more inclined to add support for core and stdlib libraries to ensure interop. I usually won't add support up-front unless I have a use case though or I'm following a ruby convention.

dkubb avatar Dec 18 '13 16:12 dkubb

Closing this for now. I may add devtools in the future, but it will need some extra rework.

dkubb avatar Jul 30 '23 05:07 dkubb