ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 208 forks source link

2.0.0 (aka 1.0.0) #238

Open hypesystem opened 8 years ago

hypesystem commented 8 years ago

:star: should not be merged :star:

This is a long-running branch that will exist while we are figuring out the exact interface of version 1.0.0 of this project.

New changes to the v1 branch should be made as separate PRs merging into v1.

The current latest alpha of 1.0.0 is 1.0.0-alpha.1. It is not recommended in production. Use instead 0.14.2, which lives on the master branch of this repository.

Preliminary TODO for 1.0.0:

Comments, feedback, etc. on this process are very welcome :smile:

eladnava commented 8 years ago

@hypesystem what do you say about removing lib/sender.js and moving all of its contents to the index.js file in the root project directory?

Seems kind of unnecessary to require 2 empty files on the way as it is today:

require('node-gcm') -> index.js -> lib/node-gcm.js -> lib/sender.js

hypesystem commented 8 years ago

@eladnava I think we are probably going to split the Sender file at some point, so it will make more sense. You are right: right now it is very silly. I would rather wait and see exactly what happens with the Sender, before deciding on the file hierachy :smile:

eladnava commented 8 years ago

@hypesystem Isn't that 'some point' now since we are basically cleaning everything we think should be cleaned up? Or would you rather wait till the end of everything else?

hypesystem commented 8 years ago

@eladnava haha yeah, I'm not saying "let's wait for a long time" --- just until we are a bit further with the new v1 interface :smile:

We have eliminated a lot of abstractions, and plan to add some more functionality to the Sender -- so it will grow quite big, and then it should be split into smaller files. After that, we can figure out how our index.js should look :smile:

hypesystem commented 8 years ago

Published 1.0.0-alpha.1 :smile:

eladnava commented 8 years ago

Awesome! :octocat: 😄

hypesystem commented 7 years ago

How will we handle users warning to send to topics fitting a condition?

eladnava commented 7 years ago

@hypesystem How do you currently detect a topic versus a registration token passed in via the second argument?

hypesystem commented 7 years ago

@eladnava Currently we do not have to. Topics are passed to the to field, just like a single registration token should be. This was done to move closer to the actual HTTP interface, where that seemed to be the distinction. With conditions, this changes slightly. I will try to come up with some idea for a solution when I am back from summer holidays.

eladnava commented 7 years ago

We could instead change the interface to allow passing in an object (but the same exact object later passed into the request, e.g. to, registration_ids, or condition). If an array is passed in, treat it like registration_ids, and if a string, set it as the to.

hypesystem commented 7 years ago

That would definitely be one option :smile: it's quite verbose, but I guess that the simple string and array options will be the common cases, and they will still be supported... :+1: