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

Fixed #4: TCP sockets for connection #7

Closed
wants to merge 4 commits into from

Conversation

sauravhiremath
Copy link
Contributor

No description provided.

Copy link
Member

@rakshith-ravi rakshith-ravi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Seems like some solid work. However, I'd like to get #5 merged before this gets merged so that we can have some CI testing done on PRs.

src/juno-node.ts Outdated Show resolved Hide resolved
src/juno-node.ts Outdated Show resolved Hide resolved
src/juno-node.ts Outdated Show resolved Hide resolved
src/juno-node.ts Outdated Show resolved Hide resolved
src/connection/unix-socket-connection.ts Outdated Show resolved Hide resolved
src/connection/inet-socket-connection.ts Outdated Show resolved Hide resolved
@rakshith-ravi
Copy link
Member

Everything else seems fine. Let's get #5 merged before we merge this.

Copy link
Member

@rakshith-ravi rakshith-ravi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert the CI parts. Those are a separate PR. Doesn't belong here

@sauravhiremath sauravhiremath force-pushed the master branch 2 times, most recently from 4e16779 to f50d4db Compare May 1, 2020 04:42
@sauravhiremath
Copy link
Contributor Author

Revert the CI parts. Those are a separate PR. Doesn't belong here

Yeah reverted the changes. But at least we know PR checks are working 😅 (The checks were successful when I added the unwanted commit to PR)

@rakshith-ravi
Copy link
Member

Can we have a new PR to target develop instead of master @sauravhiremath? We can merge it once that's done

@thebongy thebongy closed this May 1, 2020
@thebongy thebongy reopened this May 1, 2020
thebongy added a commit that referenced this pull request May 7, 2020
* feat #4: TCP sockets support added

* feat #4: Error handling for socket path

* fix #7: Import destructuring and minor fixes

* fix #7: fs renamed to fsPromises

* Update github repo link

* fix: Don't fail build if just version check fails if not publishing release

* 0.1.0 beta test release (#11)

* fix: Correct downloaded atrifact name

* Test staging release

* Update build.yml

* fix: Bump version in package.json on staging

* fix: bump version

* fix: rename tar file

* fix: Rename tar only if doesn't exist

* fix

* Removed unnecessarry await

* fix: Support passing data along with hook
Closes #16

Co-authored-by: Saurav M. H <sauravhiremath@gmail.com>
Co-authored-by: Rakshith Ravi <rakshith.ravi@gmx.com>
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.

3 participants