status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.89k stars 985 forks source link

0.9.32 mobile release #6891

Closed lukaszfryc closed 5 years ago

lukaszfryc commented 5 years ago

We will use this issue to manage 0.9.32 release.

open issues and PRs for release: https://github.com/status-im/status-react/labels/release

lukaszfryc commented 5 years ago

@chadyj can we fill description of this issue with: 1) Go to dev person who will take care of cutting the release branch and doing hot-fixes 2) QA person responsible for release testing 3) Product person to coordinate the release with marketing, etc.

Also, are we going to release next week? If yes, can we cut the release/0.9.32 tomorrow? We have group chats that we should release asap as they will minimize the impact of PFS release in the future.

Enabling PFS for 1-to-1 will be a breaking change, but if we release group chats (or pairing, or both ) next week, those clients will be already compatible, so it will give us a bit more time for users to upgrade, making the upgrade less painful.
https://github.com/status-im/status-react/issues/6882#issuecomment-441974577

chadyj commented 5 years ago

@mandrigin any comments for the above?

@jeluard Mentioned that this release needs to be out by Sunday Nov 2nd, so lets hustle!

@jeluard Is everything in develop and ready to go? What are the Kyber changes needed?

jeluard commented 5 years ago

@chadyj Some commits are still missing. Realistically the target date would be Dec 7

annadanchenko commented 5 years ago

I can be responsible for release testing and coordinating with test team except this Friday (nov 30).

@jeluard when missing commits will be ready, so we cut off the branch for testing? Ideally, we'd like to have 1st release build for regression testing tomorrow morning.

jeluard commented 5 years ago

@annadanchenko Sounds good!

mandrigin commented 5 years ago

@chadyj emm, what did you want me to comment on? 😆

chadyj commented 5 years ago

@chadyj emm, what did you want me to comment on? 😆

This comment https://github.com/status-im/status-react/issues/6891#issuecomment-441998313 Specifically who is the go-to dev person, and cutting the release tomorrow. Some feedback here would be good too.

mandrigin commented 5 years ago

Well, I can fix some bugs for this release.

annadanchenko commented 5 years ago

@mandrigin can you create 0.9.32 release build, please? Should also have version 0.9.32 in the app

chadyj commented 5 years ago

Translation string updates are ready for testing and review https://github.com/status-im/status-react/issues/6891

mandrigin commented 5 years ago

@annadanchenko done. release/0.9.32 is cut and the build is running: https://ci.status.im/job/status-react/job/release/job/release%252F0.9.32/

release/0.9.32 [status-react » release] [Jenkins]
chadyj commented 5 years ago

cc @Blockchain-Islander @pablanopete @j-zerah Take a look. If all goes well well we can release Monday or Tuesday.

annadanchenko commented 5 years ago

@corpetty @mandrigin guys, do we need to do anything extra from security pov on the release 0.9.32 built today? like review again dependencies, as https://github.com/status-im/status-react/issues/6906 is not done yet.

annadanchenko commented 5 years ago

according to @cammellos we need to cherry pick https://github.com/status-im/status-go/pull/1293 into the release if we want to have group chats in it.

If it's a low impact change on status-go side then I'm fine with it. Any opinions on it @adambabik @mandrigin @chadyj ?

chadyj commented 5 years ago

If it's a low impact change on status-go side then I'm fine with it. Any opinions on it

I'd love to see group chats #6882 in the release if this is a low impact change.

lukaszfryc commented 5 years ago

Btw, I've been checking average battery consumption in nightlies for the last 20 days and it looks stable. For measurements, I used https://github.com/status-im/status-react/pull/6692 with some reference tests like creating new account, sending message, etc. Screenshot of latest results.

chadyj commented 5 years ago

Kudos @lukaszfryc for checking battery consumption!!!

Serhy commented 5 years ago

Release 0.9.32 builds (https://ci.status.im/job/status-react/job/release/job/release%252F0.9.32/2/) has the (9999) version both on iOS and Android: Version 0.9.32 (9999); node 27700aa2

https://github.com/status-im/status-react/issues/6881 is related and it's fix already in progress: https://github.com/status-im/status-react/pull/6894

@jakubgs, @mandrigin we'll need to add it in release/0.9.32 as well in order to have app version with new format.

status-react » release » release/0.9.32 #2 [Jenkins]
corpetty commented 5 years ago

@corpetty @mandrigin guys, do we need to do anything extra from security pov on the release 0.9.32 built today? like review again dependencies, as #6906 is not done yet.

Do we have a timeframe on getting #6906 done? I don't have anything else that is incredibly pressing for this release.

mandrigin commented 5 years ago

I made it work on macOS just fine, on Linux npm behaves weirdly :( fixing that now...

Serhy commented 5 years ago

6894 has been merged into develop.

@mandrigin could we cherry pick it release also please?

annadanchenko commented 5 years ago

and https://github.com/status-im/status-react/pull/6917 is also ready to be merged and cherry picked @mandrigin

mandrigin commented 5 years ago

@corpetty @annadanchenko #6906 — let's switch to yarn instead. I'll work on it, but it shouldn't stop the release. We can check the modified package-lock file for release builds separately, if needed.

mandrigin commented 5 years ago

6894 — cherry-picked to release/0.9.32

6917 — merged to develop and cherry-picked to release/0.9.32

jeluard commented 5 years ago

FYI There's a bunch of extensions related commits that will have to be cherry picked.

jeluard commented 5 years ago

@mandrigin There are 2 branches: release/0.9.32 and releases/0.9.32. Which one is the right one?

mandrigin commented 5 years ago

release/0.9.32, there is a typo in the second one

jeluard commented 5 years ago

@mandrigin Could you delete it so that I don't mixup both ? :)

mandrigin commented 5 years ago

done

chadyj commented 5 years ago

Release notes for 0.9.32 so far are:

0.9.32

Added

Changed

Fixed

All changes

@rachelhamlin @jeluard anything you'd like to add for the new extensions work? Notes can be edited here https://notes.status.im/mobile-release-notes?edit

annadanchenko commented 5 years ago

@chadyj @mandrigin it looks to me like a release blocker https://github.com/status-im/status-react/issues/6944 if agree, please add release label to it

mandrigin commented 5 years ago

agreed

annadanchenko commented 5 years ago

adding https://github.com/status-im/status-react/issues/6903 to the release scope (as according to @yenda it's same case as reported in #6944 )

rasom commented 5 years ago

regarding https://github.com/status-im/status-react/issues/6903

Fix to make it work properly on nigthly side is relatively simple (recalculate old id, store it along with message, use both message-id and old-message-id in order to fetch the reference).

In order to make it work on both sides, we can always send an old message id as a reference id. Though IMO this will only complicate things.

The simplest way for us is to introduce breaking change in 0.9.32 and also add (again) random message-id so that we will not need to have similar issues in future.

yenda commented 5 years ago

@rasom I'm in favor of breaking change as long as we now pass the message-id with the message to avoid that type of problem the next time we change message-id generation

mandrigin commented 5 years ago

I 100% agree with @yenda. It has bitten us before, and it will bite us in the future unless we fix it now.

lukaszfryc commented 5 years ago

re breaking changes, I think we may have another one when PFS is implemented. Am I right @cammellos? If yes, maybe we should release both things at once?

rasom commented 5 years ago

@lukaszfryc we can make it compatible, and i'm going to introduce the change for this during next hours

Serhy commented 5 years ago

There is a release blocker: https://github.com/status-im/status-react/issues/6963 Which is already fixed in the scope of https://github.com/status-im/status-react/pull/6932

@mandrigin @chadyj since #6932 is not in the scope of release we could go with: a) Include #6932 in scope of release or b) Cherry-pick commit fixing issue #6963 in the release. What's the best option for us at this moment? (I'd go with (b) case)

cammellos commented 5 years ago

I'll make it easy for everyone, I will just cherry pick the small fix (it's a one line change), and then if we want to include the whole PR we can decided later

cammellos commented 5 years ago

cherry picked https://github.com/status-im/status-react/pull/6965

lukaszfryc commented 5 years ago

When https://github.com/status-im/status-react/pull/6932 is merged, it will contain fixes for #6963 and #6965.

So apart from #6903 (not a blocker?) we are almost done. We just need to re-test some areas of the app on Monday.

chadyj commented 5 years ago

@jeluard You all set?

jeluard commented 5 years ago

I’d love to have #6957 too but I am AFK. @flexsurfer can you handle that?

Le 30 nov. 2018 à 16:21, Chad Jackson notifications@github.com a écrit :

@jeluard You all set?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

flexsurfer commented 5 years ago

cherry picked #6957 https://github.com/status-im/status-react/commit/18b75226423374fac58ee96583f0fe05e1159ceb

Serhy commented 5 years ago

When #6932 is merged, it will contain fixes for #6963 and #6965.

That's fine, agree. Ideally, I'd exclude #6932 from cherry-picking, though. Because mostly it contains additional features we are about to include after release branch cut off (PR #6935 fixed original release blocker #4868)

rachelhamlin commented 5 years ago

Are we targeting Tuesday for release @chadyj?

cc @j-zerah

chadyj commented 5 years ago

Are we targeting Tuesday for release @chadyj?

Ideally yes, but could run late with testing and more fixes.

lukaszfryc commented 5 years ago

When #6932 is merged, it will contain fixes for #6963 and #6965.

That's fine, agree. Ideally, I'd exclude #6932 from cherry-picking, though. Because mostly it contains additional features we are about to include after release branch cut off (PR #6935 fixed original release blocker #4868)

TBH, I don't see a reason why we should not include #6932 in the release. It contains useful feature for group chats and is all tested. cc @Serhy @cammellos

Anyway, we need to retest some parts of the app before the regression.

Serhy commented 5 years ago

TBH, I don't see a reason why we should not include #6932 in the release. It contains useful feature for group chats and is all tested. cc @Serhy @cammellos

The reason, in a nutshell, is to follow the plan in what we about to release (the new features and bugfixes we stick to by release branch cut off). It makes sense to include fixes to outstanded regressions, but not really makes sense to include new features (what for to cut off the release branch then?)

@lukaszfryc @cammellos let's cherry-pick #6932 in release/0.9.32, this is the part we were aimed to retest anyway. But lets agree (and keep in release notes) afterwards, the rules we try to follow in relation to similar questions in future. cc @chadyj