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

Use in-slice from racket/sequence. #144

Closed
wants to merge 1 commit into from

Conversation

stamourv
Copy link

This function moved there as of v6.2.900.9.

This change is not compatible with Racket 6.2.1 and prior, which can be fixed by using version exceptions on pkgs.racket-lang.org.

This code will continue working (without this change) in subsequent Racket versions as long as the unstable-lib package is installed. This package will eventually not be part of the main distribution, and so would need an explicit dependency (which is a good idea in general). This solution in compatible with Racket 6.2.1 and prior.

Moved there as of v6.2.900.9.
@jackfirth
Copy link
Owner

See #213 (comment)

@AlexKnauth
Copy link
Collaborator

Stephen Chang (@stchang) would like to discuss this further, to remove the dependency on unstable-lib, including transitive dependencies on packages like ppict, staged-slide. Now that some time has passed and there are several racket versions with in-slice included, a version exception for the older versions wouldn't be as much of a problem.

@stchang
Copy link
Contributor

stchang commented Jan 25, 2017

Thanks. Yes, it would be nice to not have to pull in things like ppict and staged-slide just to use lens. Most things from unstable like in-slice has been moved into main Racket. There is still one use of in-pairs from unstable/sequence but this can be replaced with in-dict (see racket/unstable@c6464a3).

Otherwise, I'm also ok with maintaining a separate fork and calling it something like lens-no-unstable.

@jackfirth
Copy link
Owner

jackfirth commented Jan 25, 2017

Version 6.1.1 is about two and a half years old at this point. I'm fine with dropping support for it completely and depending on 6.3. I'd rather drop support than maintain version exceptions, as I think the Racket package ecosystem is too small for it to be worth the extra maintenance effort.

Version exceptions and which Racket version to require is a question that's come up a few times in the lens project now. @AlexKnauth, do you think it would make sense to set some rules about when a version should or should not be supported? I'm thinking something like "we'll always support the last X versions, and we'll support up to the last X+Y versions if it's not difficult and there's no features we want in newer versions".

@AlexKnauth
Copy link
Collaborator

It would be 6.2.1 and below that we would be dropping support for. For a version exception, we wouldn't have to maintain the version exception branch, so why not?

@stchang
Copy link
Contributor

stchang commented Jan 25, 2017

Great! Submitted a pull request if that makes things easier. Should the racket dependency be 6.3 or 6.2.2?

@jackfirth
Copy link
Owner

jackfirth commented Jan 25, 2017

A version exception branch that we don't add new features to will get out of sync with the docs at pkgs.racket-lang.org, which seems undesirable to me. A version exception branch that we do add new features to doubles the Git work involved in adding new features.

@stchang Version 6.2.1 is fine with me.

@AlexKnauth
Copy link
Collaborator

But 6.2.1 doesn't provide in-slice from racket/sequence. It the main branch would have to depend on 6.3, and the exception branch would be for versions up through 6.2.1.

@jackfirth
Copy link
Owner

Whoops, @AlexKnauth is right - it should be 6.3

@stchang
Copy link
Contributor

stchang commented Jan 25, 2017

Shoot, it looks like kw-make-struct still depends on unstable-lib. I'll go bother that package.

@AlexKnauth
Copy link
Collaborator

kw-make-struct relies on unstable/struct for get-struct-info. I don't think that's provided by any stable library.

@AlexKnauth
Copy link
Collaborator

AlexKnauth commented Jan 25, 2017

Though I could easily replace it with a dependency on the struct-id syntax class from Alexis King's "More Syntax Classes" package.

@stchang
Copy link
Contributor

stchang commented Jan 25, 2017

Do it!

@stchang
Copy link
Contributor

stchang commented Jan 25, 2017

On a separate but related note, would it be possible to remove the lens-data dependency of lens-common?

The dependency seems to be only due to some tests in these files:
https://github.com/jackfirth/lens/blob/11ecc2f1e1f6fab82d6cfc5b0e217b4755dfc2ce/lens-common/lens/private/compound/lazy.rkt
https://github.com/jackfirth/lens/blob/11ecc2f1e1f6fab82d6cfc5b0e217b4755dfc2ce/lens-common/lens/private/compound/zoom.rkt
https://github.com/jackfirth/lens/blob/11ecc2f1e1f6fab82d6cfc5b0e217b4755dfc2ce/lens-common/lens/private/isomorphism/compound.rkt

But since lens-data already depends on lens-common, could the tests be moved to lens-data?

@AlexKnauth
Copy link
Collaborator

Fixed by 0d1d56e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants