Git PR Merging
by Kris Borchers
Hey all, can I make a suggestion to reduce e-mail noise when merging PRs? Basically, I was the one that started doing the "thanks, landed in <sha>" messages on merges. There is no reason to add those comments when the SHA doesn't change. The only reason for those messages is to let the PR submitter know where their change landed when the SHA is different and the PR has to be manually closed.
A standard merge will keep the SHAs intact and automatically close the PR, which is the usual case for most PR merges. If you ever have to rebase a PR for someone or you squash some of their commits then this would cause SHA changes and a "landed in <sha>" message is helpful.
Also, there is usually no reason to add a "Merged <PR#>" comment. Those are either generated by GitHub when you merge or in the case described above, the "landed in" message is good enough.
Let me know if that doesn't make sense or if you have questions.
Thanks!
10 years, 9 months
Android push issue?
by Marc Sluiter
Hi all,
I had a further look into Android push, and there is a thing I don't understand,
maybe there is an issue:
In Registrar.onPostExecute() there are these lines:
ComponentName component = new ComponentName(context, AGPushMessageReceiver.class);
int status = context.getPackageManager().getComponentEnabledSetting(component);
if (status == PackageManager.COMPONENT_ENABLED_STATE_DEFAULT) {
// register AGPushMessageReceiver
}
What should this code do? Should it check if a receiver is registered already?
If yes, the problem with this is, that the if statement is always true,
regardless of whether there is a receiver registered in the manifest or not. So
it registers a second one, if there is already one in the manifest. Tested by
setting a breakpoint to the receivers onReceive() method, it's called twice for
each notification.
An alternative way to check if a receiver is registered is this:
String action = "com.google.android.c2dm.intent.RECEIVE";
Intent intent = new Intent(action);
List<ResolveInfo> resolveInfos =
context.getPackageManager().queryBroadcastReceivers(intent,
PackageManager.GET_INTENT_FILTERS);
boolean isRegistered = false;
String receiverName = AGPushMessageReceiver.class.getName();
for(ResolveInfo resolveInfo : resolveInfos){
if(resolveInfo.activityInfo.name.equals(receiverName)){
isRegistered = true;
break;
}
}
if(!isRegistered){
// register AGPushMessageReceiver
}
I can send a PR if you want to.
Furthermore I'm not sure if it really makes sense to register a
BroadcastReceiver for push notifications programmatically. The reason is, that
this receiver is tied to the lifecycle of the app. That means after a reboot no
messages are received until the app is started, and when Android destroys the
app after some inactive time, no messages will be received anymore too. See the
note in
https://developer.android.com/reference/android/content/Context.html#regi...,
android.content.IntentFilter):
"..the lifetime of such a registered BroadcastReceiver is tied to the object
that registered it."
I confirmed this by commenting out the receiver in the PushEE demo app manifest.
What do you think?
Regards,
Marc
10 years, 9 months
Integration test execution policy on Aerogear
by Karel Piwko
Hi All,
this week we added integration tests to Unified Push Server[1]. There are 3 new
profiles: as711-managed, as711-remote, eap610-managed. Managed profiles are
intended to run in CI, everything is managed by tests. Remote profiles are for
test development - you start AS on your own and save ~10 seconds in each test
suite execution.
Now we need to reach the agreement what are *the right* defaults. Here are the
options:
1/ Now: Run integration tests in Maven verify phase by default. as711-managed is
activated by default
This means that 'mvn package' is not affected. 'mvn install' will run the
tests, starting its own AS7, that is downloaded if not available locally. So far
tests take like 2 minutes, 10 minutes Maven trying download Internet minus
porn for the first run. The problem is that you have to ensure that AS7.1 ports
(8080, 9999) are free every time you run mvn install.
2/ Keep 1/ but add an offset to ports (e.g +100).
This will reduce the port conflicts if you are running AS instance somewhere
else. Or if you are listening to mp3 streams on 9999 ;-). This will complicate
remote scenario, e.g. you'd need to start remote instance with offset. Does not
affect CI.
3/ Skip integration tests by default. No "container" profile is activated by
default, skipITs=true by default. Activate container profile in Travis and QE
machines only.
Would need to activate a Maven profile for test development. Travis will
still bug developers if something fails ;-)
4/ Something else?
WDYT? I'd like to reach an agreement so that the same policy could be applied
across all Aerogear modules.
Thanks,
Karel
[1]
https://github.com/aerogear/aerogear-unified-push-server/commit/e1651edb6...
10 years, 9 months
Some UnifiedPush Tests
by Matthias Wessendorf
Hi,
over the weekend I tested the UnifiedPush server, deployed on my private
OpenShift account. I sent 30k+ messages from my Mac to UP on OpenShift.
Messages were received on my iPad and my Galaxy SII.
* 10k Messages from JUnit (via Java Client). Message Rate: every 5 seconds.
* 10k Messages from JUnit (via Java Client). Message Rate: every
2.5 seconds
* 10k Messages from JUnit (via Java Client). Message Rate: every
1.5 seconds
Besides that a cron job was submitting messages to OpenShift (using cURL)
every minute.
The system worked fine on OpenShift, and the iOS/Android apps were able to
receive the messages just fine.
When I submitted 550 async jobs (using cURL) the apps had issues to display
all of the messages, as they came in very quick. However this is a stupid
test scenario anyways, as push is not for chats :-)
The OpenShift based deployment of UnifiedPush worked fine for all of these
tests.
Currently the cron job is still running and will keep it running.
Greetings,
Matthias
--
Matthias Wessendorf
blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf
10 years, 9 months
Git Merge Policy Review
by Douglas Campos
Since this is the second time I'm facing the same dillema, I think it's
worthwhile to discuss this here.
We have been doing the --no-ff merges for everything, but I just found
two cases where I really think we should stick to fast-forward merges:
single-commit PRs and PRs to feature branches.
Thoughts on this? Any other comments/suggestions wrt git workflows we
use?
--
qmx
10 years, 9 months
SimplePush Polyfill Changes
by Kris Borchers
Hey all,
Before I make a change to this polyfill I wanted to make sure there were no concerns or objections. Basically, what I want to do is change the usage so that calls to navigator.push.register() or navigator.push.registrations() (not yet added) would have to be done after a connection is established. Currently, a user can call either of those methods at any time which causes headaches when trying to account for registrations that happen before the connection and also register previously used channels stored in localStorage. Just as an FYI, this is not an issue for Mozilla because they already know they have a connection when their JS executes because the browser creates the connection during startup. I have done a LOT of work to try to account for race conditions which may occur and in the process added a LOT of extra code. And even with all of that work, I'm still not 100% sure that all race conditions have been accounted for once this code is used at scale with lots of connections.
So what am I suggesting? I would like to require our users to create a connection to the SimplePush server and do their interactions with that server in the success (or onConnect or what ever we want to call it) callback. After that, all code would be identical to the Mozilla APIs. Then, we could just write a brief guide or tutorial on transitioning code between our implementation and Mozilla's to highlight the small difference.
Thoughts?
10 years, 9 months
