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

Added definitions for v1.3.5 of the socket.io lib #4894

Merged
merged 11 commits into from Jul 22, 2015

Conversation

Projects
None yet
3 participants
@divillysausages
Copy link
Contributor

divillysausages commented Jul 11, 2015

I've moved the old definitions to legacy/socket.io-1.2.0 etc. The tests are the same, as the actual implementation hasn't changed.

When running the tests for the server lib, I get the following errors:

../node/node.d.ts(260,26): error TS2304: Cannot find name 'DataView'.
../node/node.d.ts(274,21): error TS2304: Cannot find name 'Map'.
../node/node.d.ts(283,21): error TS2304: Cannot find name 'Set'.
../node/node.d.ts(293,25): error TS2304: Cannot find name 'WeakMap'.

Which I guess is linked to Issue #4249? In any case, the errors with the node definitions, not the socket.io ones.

@divillysausages

This comment has been minimized.

Copy link
Contributor

divillysausages commented Jul 11, 2015

Hm, the two errors in Travis are:

socket.io/socket.io.d.ts(762,51): error TS1005: ';' expected.
socket.io/socket.io.d.ts(762,70): error TS1005: ';' expected.

but the line in question is:

broadcast( packet: any, opts: { rooms?: string[], except?: string[], flags?: {[flag: string]: boolean} } ):void;

51 and 70 are just after the "t" in "string" - am I missing something?

* - except: A list of Socket IDs to exclude
* - flags: Any flags that we want to send along ('json', 'volatile', 'broadcast')
*/
broadcast( packet: any, opts: { rooms?: string[], except?: string[], flags?: {[flag: string]: boolean} } ):void;

This comment has been minimized.

@vvakame

vvakame Jul 13, 2015

Member

you should use broadcast( packet: any, opts: { rooms?: string[]; except?: string[]; flags?: {[flag: string]: boolean;}; } ):void;. not comma.

divillysausages added some commits Jul 17, 2015

Merge branch 'master' of https://github.com/borisyankov/DefinitelyTyped
Conflicts:
	socket.io-client/socket.io-client.d.ts
@divillysausages

This comment has been minimized.

Copy link
Contributor

divillysausages commented Jul 17, 2015

I fixed the problem with the broadcast() method, though I guess someone edited the socket.io-client file in the meantime. There was a merge conflict with my changes, but my changes encompass the edit, so everything should be fine

@divillysausages

This comment has been minimized.

Copy link
Contributor

divillysausages commented Jul 17, 2015

To be more precise, there was a merge conflict when I updated my forked repo, but the edits I've made should be fine

@vvakame

This comment has been minimized.

Copy link
Member

vvakame commented Jul 18, 2015

please fix travis ci.

@divillysausages

This comment has been minimized.

Copy link
Contributor

divillysausages commented Jul 18, 2015

Finally :) Sorry about that; I'm a bit new to pull requests and travis.

On the contributions page it says: "If you need many attempts to fix errors then please flatten the history and clean the commit message." - need me to do this? Also, how do I do it?

// Project: http://socket.io/
// Definitions by: PROGRE <https://github.com/progre/>
// Definitions by: divillysausages <https://github.com/divillysausages/>

This comment has been minimized.

@vvakame

vvakame Jul 19, 2015

Member

please keep original author.

This comment has been minimized.

@divillysausages

divillysausages Jul 19, 2015

Contributor

OK, sorry, I thought the author was updated with each revision. Want me to change it or leave it for @progre when he reviews the PR?

This comment has been minimized.

@vvakame

This comment has been minimized.

@divillysausages

divillysausages Jul 20, 2015

Contributor

Just to clarify - we keep the original author, even if the definitions are for a different version of the lib?

I'll revert the line when I get back to my computer tonight. Tks

This comment has been minimized.

@vvakame

vvakame Jul 20, 2015

Member

yep. author is reviewer and 1st class user.
please add your name below progre's name.
like this. https://github.com/borisyankov/DefinitelyTyped/blob/master/jquery/jquery.d.ts#L3

This comment has been minimized.

@divillysausages

divillysausages Jul 20, 2015

Contributor

Ah, ok, will do, thanks!

This comment has been minimized.

@divillysausages

divillysausages Jul 20, 2015

Contributor

Fixed!

@vvakame

This comment has been minimized.

Copy link
Member

vvakame commented Jul 19, 2015

@divillysausages np.
@progre could you review this PR?

@progre

This comment has been minimized.

Copy link
Contributor

progre commented Jul 19, 2015

@vvakame LGTM

vvakame added a commit that referenced this pull request Jul 22, 2015

Merge pull request #4894 from divillysausages/master
Added definitions for v1.3.5 of the socket.io lib

@vvakame vvakame merged commit 4050690 into DefinitelyTyped:master Jul 22, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vvakame

This comment has been minimized.

Copy link
Member

vvakame commented Jul 22, 2015

@progre thanks!
@divillysausages thanks mate!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment