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

Move to @types #116

Closed
blakeembrey opened this issue Dec 1, 2016 · 27 comments
Closed

Move to @types #116

blakeembrey opened this issue Dec 1, 2016 · 27 comments

Comments

@blakeembrey
Copy link
Member

Hi @donnut. Wanted to open this up for feedback from you, but would be interested in moving this repo into @types with the rest of the projects? I'd love to help out on this definition where possible and I'm happy to add you to that organisation, if you'd like (with or without moving this repo).

@KiaraGrouwstra
Copy link
Member

I can't speak for @donnut, but I'm under the impression he's become less active here. I'm currently actively using Ramda with Angular 2, so I have a vested interest in helping out to maintain these type definitions. I've tried to improve on the existing typings and merged all commits across the network in as well. If possible, I'd be interested in helping out with a repo at @types as well.

@KiaraGrouwstra
Copy link
Member

KiaraGrouwstra commented Dec 5, 2016

By the way, a related discussion popped up at the main repo, about the idea of merging typings into the main repo. How would you view this consideration of placing type definitions with @types vs. with Ramda's main repo @blakeembrey?

P.S.: since you'd be more knowledgeable on the TypeScript side, one thing I'd be interested in getting opinions on would be how to handle currying more effectively.
So we'd had some helpers for that, and the current approach had kind of been to, e.g. for an arity-4 function, do the type definition at least 4 times:

  • (a,b,c,d) => Res
  • (a,b,c): (d) => Res
  • (a,b): CurriedFn2<c,d,Res>
  • (a): CurriedFn3<b,c,d,Res>
    ... simplifying for brevity.
    In case there would be two distinct definitions for a function (e.g. string and array versions), this would further multiply.

I felt this was a bit of a pain point currently, and had wondered if we could just do one definition in a single curried definition like that, although currently it appeared parameterized functions (-> using generics) failed to parse for me that way.
Moreover, TypeScript also did not seem to like the idea of having regular 'function' versions side-by-side as well (needed if definitions using generics have to go the traditional route), while I suppose this curried route would currently also lose parameter names (hopefully improved on in the WIP variadic kinds?).

If you'd have any suggestions on how we could improve maintainability there, it'd be much appreciated. :)

@blakeembrey
Copy link
Member Author

How would you view this consideration of placing type definitions with @types vs. with Ramda's main repo @blakeembrey?

The best place for type definitions is always with the repo - it enables us TypeScript users to use it immediately without needing to do anything else.

I'll try to help out on the other issues when I get a chance, but I can't currently think of a good solution. I like the generic approach also (at least until variadic kinds land) but it has a few side effects - just need to wait for variadic kinds.

@KiaraGrouwstra
Copy link
Member

Thanks for the input. Looking forward to that. :)

@KiaraGrouwstra
Copy link
Member

With everyone's preference so far seeming to go toward a merge into the main ramda repo, I'm inclined to think this can be closed unless that plan were to fail somehow. If so we can reopen again. :)

@KiaraGrouwstra
Copy link
Member

We're currently worrying about getting constrained in versioning in case of a merge with Ramda (I think we might appreciate the freedoms of a faster publishing cycle), so your proposal might still remain relevant.

One question having the repo in @types would raise for me though would be w.r.t. where users should file their typing-related issues, especially since for the purpose of github's linking of issues to commits/PRs afaik it'd be desirable to have typing issues in the same repo as the typings code itself.
Or would the @types sub-repo be constructed as a mirror of the current repo? I'm not very knowledgeable of best practices there, so I'd glad to see your take there.

@blakeembrey
Copy link
Member Author

We'd just add whoever can move the repo to the @types repo and they can move the repo into @types. Redirects would work through GitHub. You can browse through @types repos to see, but it's maintained just like here - just it's in an org where dozens of others can also pick up on maintenance if needed.

@KiaraGrouwstra
Copy link
Member

Thanks. I think my own concerns are addressed.

@KiaraGrouwstra
Copy link
Member

There was one compatibility-breaking feature request I'd suggested to relegate to a branch (see #113). To my understanding from the Types guidelines it'd be possible to have that survive DefinitelyTyped for end-user consumption if using a branch / folder, right?

@blakeembrey
Copy link
Member Author

Sure, a branch works. Whatever makes it easier for you to maintain, I've been doing a little bit of branching when releasing a new major version to be able to update bugs in previous major versions, though I don't find that to be an issue often.

In terms of DefinitelyTyped, we're currently having to manually make PRs to it (if you're maintaining a copy there) until microsoft/types-publisher#4 lands (we're all stuck waiting for that sadly).

@KiaraGrouwstra
Copy link
Member

Alright, cool. Wouldn't be the first addition on my wishlist, so oh well. :)

@KiaraGrouwstra
Copy link
Member

@blakeembrey: could you perhaps send @donnut an invite as well? I fear the issue lists are all in the current repo, which I imagine only he would the ability to move over.

@KiaraGrouwstra
Copy link
Member

@blakeembrey: hey, just pinging again in the event you'd missed this earlier; if there are any complications feel free to bring them up.

@blakeembrey
Copy link
Member Author

@tycho01 Sorry, I had sent the invite to @donnut when you asked about it 😄 I just double checked again now.

@KiaraGrouwstra
Copy link
Member

Guess it's in his hands now then, thanks again! :)

@KiaraGrouwstra
Copy link
Member

Few things we wanted to confirm:

  • if people still have/use the old repo link, that'd get redirected, right?
  • would such redirects still work despite the subsequent rename to npm-ramda?

@blakeembrey
Copy link
Member Author

Yes to 1 and 2, it's worked in the past without issue.

@KiaraGrouwstra
Copy link
Member

KiaraGrouwstra commented Dec 24, 2016

Still waiting for Erwin to perform the move, but I just had an unrelated question pop up I wouldn't know well where else to ask.

I know how we were testing now, and felt some concern about it, but now found that the situation for Lodash at DefinitelyTyped appeared no different. From what I can see, this way of testing now helps test for cases where typings are improperly strict (-> error), but not for cases where the typings are improperly vague (-> no compilation error, all gone at run-time so no good way to test by writing run-time tests that I'm currently aware of). Is this just me, or is that just the current state of TS definition testing?

@blakeembrey
Copy link
Member Author

@tycho01 I may have misinterpreted, but it sounds like you're asking about testing runtime vs definition time? If so, you may be able to find up some old Typings threads on this, but from mine and @types perspective we've always tested both the runtime and compile time using something like ts-node and tsc with noEmit enabled. You can see it https://github.com/types/npm-bcrypt/blob/35f8721dbf2072bc65f1495f58905aaba2158d9b/package.json#L9. We also have a few of repos that have been created or moved into Typings that don't follow this pattern, but in general it's what I'd recommend. There's some basic guidelines I added a little while back here: https://github.com/types/guidelines.

If that wasn't what you were asking, let me know 😄 I believe DefinitelyTyped has always just tested compile time using tsc.

@KiaraGrouwstra
Copy link
Member

Hm. If TS infers a function to return any rather than number, can we make that a failing test?

@blakeembrey
Copy link
Member Author

I think it'd be easy enough to write a little script for it. Just run tsc for x.pass.ts and x.fail.ts and make sure the error codes are correct each execution.

@KiaraGrouwstra
Copy link
Member

Interesting approach :), I hadn't quite thought of that. Thanks!

@donnut
Copy link
Collaborator

donnut commented Dec 26, 2016

Move the repo, as you noticed ;)
@blakeembrey, can you rename it to npm-ramda?

@blakeembrey
Copy link
Member Author

@donnut Done! 👍

@KiaraGrouwstra
Copy link
Member

@blakeembrey: you mentioned PRs to DefinitelyTyped to publish. That seems a little different from the PRs I'm familiar with in the sense this would only be part of their repository. Might you have any reference on how to do this?

@blakeembrey
Copy link
Member Author

@tycho01 They have an automated script watching DefinitelyTyped to re-publish packages on changes. See microsoft/types-publisher#4 for information where we're trying to get redirects supported.

@KiaraGrouwstra
Copy link
Member

Okay. Until then, I suppose I'm okay even without being able to get it updated -- still a pretty big TS issue severely hampering usefulness of the Ramda typings anyway.

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

No branches or pull requests

3 participants