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

Registrations #15

Merged
merged 9 commits into from
Nov 5, 2014
Merged

Registrations #15

merged 9 commits into from
Nov 5, 2014

Conversation

secondsun
Copy link
Contributor

Needs AeroGear Android Core
Should mvn clean install.

Need to update quickstart for more demo testing.

Helloworld demo : https://github.com/secondsun/aerogear-push-helloworld/tree/registrations

@secondsun
Copy link
Contributor Author

@danielpassos @cvasilak care to take a look?

@secondsun secondsun force-pushed the registrations branch 2 times, most recently from ab15920 to 67e8562 Compare October 8, 2014 03:02
@matzew
Copy link
Contributor

matzew commented Oct 8, 2014

@secondsun what's the JIRA for this?

@secondsun
Copy link
Contributor Author

* Default lifespan (7 days) of a reservation until it is considered
* expired.
*/
public static final long REGISTRATION_EXPIRY_TIME_MS = 1000 * 3600 * 24 * 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the default lifetime for a registration.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is the default lifetime for a registration.

for a registrationId on GCM? Does that mean after 7 days, I get a new
registrationID?

On Wed, Oct 8, 2014 at 3:05 PM, Hoyt Summers Pittman <
notifications@github.com> wrote:

In
aerogear-android-push/src/org/jboss/aerogear/android/impl/unifiedpush/AeroGearGCMPushRegistrar.java:

public class AeroGearGCMPushRegistrar implements PushRegistrar {

 private static final Integer TIMEOUT = 30000;//30 seconds
  • /**
  • \* Default lifespan (7 days) of a reservation until it is considered
    
  • \* expired.
    
  • */
    
  • public static final long REGISTRATION_EXPIRY_TIME_MS = 1000 * 3600 * 24 * 7;

It is the default lifetime for a registration.


Reply to this email directly or view it on GitHub
https://github.com/aerogear/aerogear-android-push/pull/15/files#r18580331
.

Matthias Wessendorf

blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf

@matzew
Copy link
Contributor

matzew commented Oct 8, 2014

running mvn clean install works only if I have an online device connect to my machine...

@secondsun
Copy link
Contributor Author

@matzew
Yes that is true for literally every one of the android projects.

@sebastienblanc
Copy link
Member

@secondsun @matzew Maybe it should be added to http://aerogear.org/docs/guides/aerogear-android/how-to-build-aerogear-android/ that at least an emulator must be running to build.

@secondsun
Copy link
Contributor Author

@sebastienblanc Or you can just build the library without running the tests?
Anyway, updating the docs is on our jira list.

@matzew
Copy link
Contributor

matzew commented Oct 8, 2014

why not launch the integration tests on a specific profile, which is not enabled by default?

That's what we did on the server-side as well (arquillan even got moved into its own repo)

@secondsun
Copy link
Contributor Author

@matzew Because that doesn't work. If you can make it work feel free to submit a PR.

@matzew
Copy link
Contributor

matzew commented Oct 8, 2014

oh, wow. too bad :)

@secondsun
Copy link
Contributor Author

@matzew So it seems I'm a bit of a liar. The docs on the maven plugin are old and it supports -Dandroid.test.skip=true now. (the docs tell you to use a different flag which doesn't work. Ergo my earlier angst)

@secondsun
Copy link
Contributor Author

I've added a helloWorld branch to the PR description (https://github.com/secondsun/aerogear-push-helloworld/tree/registrations)

@@ -15,8 +16,8 @@ env:

before_install:

- echo no | android create avd --force -n test -t $ANDROID_TARGET --abi $ANDROID_ABI
- emulator -avd test -no-skin -no-audio -no-window &
- echo no | android create avd --force -n test -t 'Google Inc.:Google APIs:19' --abi $ANDROID_ABI
Copy link
Collaborator

Choose a reason for hiding this comment

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

$ANDROID_TARGET did not work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would have a problem with the path to the things later on.
Link to things : https://github.com/secondsun/aerogear-android-push/blob/registrations/.travis.yml#L32

@danielpassos
Copy link
Collaborator

✔️ Register
✔️ Receive push message

But I was thinking... Is not time to we kill all http duplicated here and delegate that to pipe module? Wdyt?

@secondsun
Copy link
Contributor Author

@danielpassos Done!

@danielpassos
Copy link
Collaborator

Travis don't like this because we need update support library version. I'm working on update android bom

For more information see: https://travis-ci.org/danielpassos/aerogear-android-push#L1442

@danielpassos
Copy link
Collaborator

I'd like to update Android bom and pipe to make travis happy before merge it

  1. Update needs to compile pipe module
  1. Pipe changes to make travis happy
  1. Pipe travis happy

@matzew
Copy link
Contributor

matzew commented Oct 16, 2014

@danielpassos agreed on the bom

@abstractj
Copy link
Contributor

@danielpassos @secondsun only to avoid my itching merge finger. I will close it until we update and release the bom. Feel free to reopen when we think it's "mergeable"

@abstractj abstractj closed this Oct 21, 2014
@danielpassos
Copy link
Collaborator

Since we are with some travis problem[1] I'm reopening and about to merge it.

@secondsun Could you rebase/update that?

[1] http://aerogear-dev.1069024.n5.nabble.com/aerogear-dev-Travis-problem-with-Android-Lollipop-td9706.html

@danielpassos
Copy link
Collaborator

Hi @secondsun

  1. Wdyt about remove PushType?
  2. Maybe move AeroGearGCM* for the impl package? Not sure about that

@danielpassos
Copy link
Collaborator

@qmx Could you review that?

@qmx
Copy link
Contributor

qmx commented Nov 4, 2014

@secondsun could you please do a brief git history review? I think those duplicated commit messages can be merged into one, and the cleanup-like ones into a bigger cleanup commit.

@danielpassos
Copy link
Collaborator

👀

@danielpassos
Copy link
Collaborator

The code looks good to me but I'd like to test it using aerogear-push-helloworld first.

@danielpassos
Copy link
Collaborator

The logic of Registrations was moved to RegistrarManager maybe kill Registrations class?

<artifactId>aerogear-android-pipe</artifactId>
<version>${aerogear.android.pipe.version}</version>
<type>aar</type>
</dependency>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move dependencies to library instead of root pom like we did with the other modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case it is correct here because the tests use -core for some things.

@danielpassos
Copy link
Collaborator

We also need to make travis happy. I think travis don't like Google Inc.:Google APIs:19 anymore

@danielpassos
Copy link
Collaborator

I've tested it using aerogear-push-helloworld and works like a charm.

@secondsun
Copy link
Contributor Author

Per @qmx did a big rebase

@qmx
Copy link
Contributor

qmx commented Nov 5, 2014

yay! Will review in a bit :)

@qmx
Copy link
Contributor

qmx commented Nov 5, 2014

👍 apply the formatter and ship it :)

@abstractj abstractj merged commit 6a9efa3 into aerogear:master Nov 5, 2014
@abstractj
Copy link
Contributor

@secondsun landed

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