Project

General

Profile

Emulator Issues #9601

Analytics for 5.0

Added by JosJuice over 3 years ago. Updated over 3 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
% Done:

0%

Operating system:
N/A
Issue type:
Feature request
Milestone:
Current
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:

Description

Short summary of what happened on IRC: delroth said that he wanted to merge analytics before 5.0 so that we can get data on users who are using on 5.0. This led to opposition for breaking the feature freeze (mainly from me), but it also received support from different people. Half an hour ago, delroth ended up moving the role of release manager to Fog, who decided that the release will be blocked until analytics are merged.

I personally think breaking the feature freeze this late is a ridiculous idea. We're either going to have a risk of regressions, a significant delay in the release, or both. I'm also concerned that there might not be enough time to discuss what the UX should be like to work well. If this is so important, it should have been done before the feature freeze. I agree that there is a big point in having analytics in 5.0, but the release complications aren't worth it.

History

#1 Updated by JosJuice over 3 years ago

Also, we can't get the new strings translated to all the languages that already have 100% completion in a reasonable time. Having untranslated strings that explain what happens to users' data is definitely not ideal. I can accept pushing strings to Transifex if necessary (breaking the string freeze), but the effect on the UX by adding new GUI strings close to the release is worrying.

#2 Updated by phire over 3 years ago

I love the idea of analytics and it's something I've wanted for ages. But I think it's something we should push into Dolphin nearer to the beginning of a release, not at the last-possible-moment.

I mean, I've just fixed the last regression and we are pretty much ready for release. I want to get back to merging new features, like super-awesome-ultimate-hybrid-xfb.

#3 Updated by phire over 3 years ago

A more conservative idea:

Put analytics in a 5.1 release. Release 5.0 next week, push the new analytics feature into master. In in a month or two we backport analytics onto the 5.0 branch along with a few other simpler fixes, any regressions we missed and maybe a feature or two and as many .ini updates as we want to create the 5.1 release.

This still gets analytics into a stable branch sooner rather than later and it doesn't hold up any work.

#4 Updated by Helios over 3 years ago

Pretty much what phire said in his first comment. Against it. Want it, but not this close to release. I even understand why this would be worth breaking freeze, if it were, say, 3 months ago when we still had 10+ blockers. Doing it now is 10 kinds of blegh.

That said, I don't care anymore. The mess of a "conversation" burnt me out and I don't particularly care what happens in regards to delaying further for this.

If Fog wants to delay, cool. If he doesn't, cool. Whatever.

#5 Updated by BhaaL over 3 years ago

In an ideal world, I'd agree with Jos - potentially destabilizing an otherwise good candidate for release with something like that shouldn't happen during a feature and/or code freeze; the deadline is past. Even though it sounds like a feature we should absolute have so we know whats going on in our user base and how we could improve their experience with Dolphin.
But at the same time, I also know the pains of things not working as they should, and last-minute changes that absolutely need to be done (in fact, I just did that at work the other day, even worse during code freeze - but with some rigurous focused testing we somewhat made it...far from the ideal world everyone wishes for, but thats reality unfortunately).

Is analytics one of those things to be allowed in this late for the party? Not sure. Might need to see the PR first to make my mind up.
If possible, I'd rather push it to another, soon-ish afterwards release; even though we explicitly said "no" to a 5.0.1 patch/hotfix release. Maybe a 5.1? /edit: ok, thats what has been suggested while I was typing. All for that path, if we can.

#6 Updated by JMC4789 over 3 years ago

I need some more time to put together release assets, so, we're not delaying the 5.0 release for this yet. We're delaying it on the PR side.

#7 Updated by JosJuice over 3 years ago

The more I think about this, the more convinced I am that releasing analytics in 5.0 without enough translations is very bad. Sending off unknown data to a server is a great way to make some users distrust us, and the data we're sending will for all purposes be unknown as long as the only information about it is in a language that the user can't read. The user will simply get an unreadable pop-up (on the first boot! Not a good first impression) and will be forced to make a choice without understanding what it means.

So is it possible to get enough translations for this? Well, I'm not going to say that it's impossible, but there are definitely problems with it. First of all, we've already told the translators that there will not be any more new strings before 5.0. We could make a new announcement saying that we have new strings again, but breaking the promise like that can come off as rude. Second, it's going to take time, quite simply. When we were having the proper 5.0 string freeze, translators had about two weeks to finish translating. From what I could see, that amount of time was just right. You should not assume that it will take any shorter than that. We might have a bit less new text than in the previous push (though it's not a huge difference), but the time it takes to translate the strings isn't everything. It's also about translators finding time in their daily schedules to translate, which we have no control over. In fact, it may take even longer than last time because people are expecting to not get more strings before 5.0 just like we told them. (And also possibly because people are on vacations.) We could probably get translations for Spanish and three other languages or so pretty fast – there are a few translators that are very quick at handling new strings – but I don't believe that that's good enough.

Fog (since you're the release manager now), I want to remind you that the reason we have a string freeze is exactly so that this situation doesn't happen. I made this as clear as I could when announcing the string freeze, and I specifically made sure that delroth was aware that there was going to be a string freeze and that he could send any concerns about it to me. Trying to push this the last minute without anything having been mentioned back when the string freeze was made is frankly irresponsible to both me as a translation manager and to the translators. It would've been very simple for delroth to have told me that he wanted analytics in 5.0 back when I asked him about the string freeze, but no such thing happened, and now we've ended up with this mess.

Finally, the analytics PR is not trivial, neither in code, UX nor testing. You can't get this PR done in a day or so. (If you do, I really am worried about what will happen to the quality of the 5.0 release.) Please keep in mind that the time it takes to make a good PR and get it merged is in addition to the time required for translations.

#8 Updated by delroth over 3 years ago

Finally, the analytics PR is not trivial, neither in code, UX nor testing. You can't get this PR done in a day or so. (If you do, I really am worried about what will happen to the quality of the 5.0 release.) Please keep in mind that the time it takes to make a good PR and get it merged is in addition to the time required for translations.

Good news, the PR was initially posted yesterday evening, reviewed during the night, now ported to Windows today. All builds are green, I don't think I've seen any new review comments yet, and JMC has started playing with it for testing (we found a few minor issues, but nothing like a crash or anything, just data not being reported properly on the first run of the software).

I'm also quite happy of the quality of the code. It should be fairly robust. I'll encourage you to have a look!

Regarding translation, we're talking about 4-5 extra strings, 2 of which are fairly long (message box + tooltip). Still, that's 5min of translation work per language.

#9 Updated by Helios over 3 years ago

FWIW we managed to find those minor issues within an hour of testing.

I'm concerned about what we'll find with more.

#10 Updated by delroth over 3 years ago

I'll challenge you.

#11 Updated by JosJuice over 3 years ago

Good news, the PR was initially posted yesterday evening, reviewed during the night, now ported to Windows today. All builds are green, I don't think I've seen any new review comments yet, and JMC has started playing with it for testing (we found a few minor issues, but nothing like a crash or anything, just data not being reported properly on the first run of the software).

I'm also quite happy of the quality of the code. It should be fairly robust. I'll encourage you to have a look!

That would definitely be enough for me if this PR had been at a regular time, but that's not what this PR is. Doing a change like this right before the release requires extra caution.

Regarding translation, we're talking about 4-5 extra strings, 2 of which are fairly long (message box + tooltip). Still, that's 5min of translation work per language.

Like I said, the amount of text is not the only factor. Around the proper 5.0 string freeze, some languages sat with no progress for a long time before the translators did anything at all. We can't expect the translators to be available for translation all the time, so we will need to wait regardless of how little new text there is.

#12 Updated by mbc07 over 3 years ago

Well, given the circumstances, I personally think it's a good trade (feature freeze vs analytics) when you think of the benefits it'll give us. Regarding the quality of the release, I disagree it would lower the overall quality, the worst that can happen is analytics not working as intended, but even on this case, it wouldn't affect the overall stability of the emulator to the end user since they wouldn't even notice something is wrong and it doesn't really change emulation code (after all, it just gathers some strings and info and upload them to the server).

And about the translations, if the new strings are already finished, just push them to Transifex as soon as possible if getting them ready is a priority for 5.0 release (I can say in name of the Brazilian Portuguese team that this won't be a problem at all given the number of new strings being very small compared to the entire emulator translation we reviewed in the previous deadline)...

#13 Updated by Helios over 3 years ago

Why wasn't this done 3 months ago then? It's not like delroth didn't want analytics then. He did.

#14 Updated by JosJuice over 3 years ago

  • Subject changed from Analytics to Analytics for 5.0
  • Status changed from Questionable to Fixed

#15 Updated by Aestek over 3 years ago

I cannot build dolphin with these changes unless I force libCURL to external in CMakeLists.txt, here is the error I get :

/home/aestek/dev/dolphin-emu/Source/Core/Common/Analytics.cpp: In constructor ‘Common::HttpAnalyticsBackend::HttpAnalyticsBackend(const string&)’:
/home/aestek/dev/dolphin-emu/Source/Core/Common/Analytics.cpp:207:26: error: ‘CURLOPT_SSL_ENABLE_ALPN’ was not declared in this scope
   curl_easy_setopt(curl, CURLOPT_SSL_ENABLE_ALPN, false);
                          ^

I run Ubuntu 14.04 (old ik, do dolphin support this OS ?) with curl 7.35.0 (x86_64-pc-linux-gnu) libcurl/7.35.0 OpenSSL/1.0.1f zlib/1.2.8 libidn/1.28 librtmp/2.3

#16 Updated by JosJuice over 3 years ago

Please create a new issue report for it instead of commenting on this closed one.

Also available in: Atom PDF