-
Notifications
You must be signed in to change notification settings - Fork 979
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
Use new status-go and rename NotifyUsers #7294
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (14)
|
@annadanchenko @asemiankevich is it possible to do a quick test on this PR without waiting for reviews? We need to implement the change from status-go ASAP. |
sure, no changing fleets? maybe i can run e2e for this too? @pombeirp |
@pombeirp, @asemiankevich all tests are failed against this PR due to super long (2 minutes) account creation, detailed results will be posted in ~10 minutes |
@pombeirp I can confirm that creating account on iOS and Android takes very long time, so it's not e2e fault but something is wrong with build logcat contains
|
Same with release Android 7 (Samsung J7) - account. |
2% of end-end tests have passed
Failed tests (57)Click to expand
Passed tests (1) |
status-go has one commit that breaks it, right.
|
@annadanchenko thanks a lot for this log, it helped a ton! |
@pombeirp @annadanchenko I will build a new release now, update this PR and it should fix the issue |
@annadanchenko @pombeirp @antdanchenko updated, lets see how it works now |
jenkins/prs/ios — This commit cannot be built @mandrigin @jakubgs |
@annadanchenko my mistake, sorry |
it is fixed now |
I have another bugfix , could you please hold on merging in case the test is ok? https://github.com/status-im/status-go/pull/1354/files , sorry for the inconvenience
|
on iOS build 4 and Android build 3 can create account |
@cammellos do you mean if test with creating account is ok or if we complete testing of this PR and find no more issues? |
@annadanchenko if you complete testing for this PR and find no issues, I would like to add a commit for status-go on top (it's a small fix), fixing the ios issue |
There is no Group Chat visible on iOS device using who upgraded from 0.9.32 build on to this PR build (https://i.diawi.com/mb6d7z) and user who uses 0.9.32 Release version. Steps to reproduce:
Actual result: None of messages are received by both iOS and Android user. They can't communicate in GroupChats. If to create new Group Chat by any of iOS and Android - new GroupChat also won't appear on Home screen on any side. |
@pombeirp desktop, I just tried on linux, I think a commit has been lost while force pushing |
Same for Mac OS. |
It's a config change, |
src/status_im/utils/platform.cljs
Outdated
@@ -28,8 +28,7 @@ | |||
(def platform-specific | |||
(cond | |||
android? android/platform-specific | |||
ios? ios/platform-specific | |||
:else desktop/platform-specific)) | |||
ios? ios/platform-specific)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this change does...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch changed wrong line!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, mistakenly commited that change
c82d832
to
af9797e
Compare
we are having some jenkins trouble, can't build, @jakubgs is looking into it |
af9797e
to
c0071af
Compare
@cammellos tested https://ci.status.im/blue/organizations/jenkins/status-react%2Fprs%2Fmacos/detail/PR-7294/15/artifacts build and got stuck in creating account after set up a password. |
@churik thanks, rebuilding now, I will test it and let you know |
85e26a8
to
25ac147
Compare
@annadanchenko it should be fixed in the last build |
thanks @cammellos indeed account can be created fine on new Windows build https://ci.status.im/job/status-react/job/prs/job/windows/job/PR-7294/20/ |
OK, Linux build is finally green. |
Update: can reproduce in nightly, #7332 I'm testing on recovered account, real Android 9 device, build 25. Can also reproduce on fresh account
Steps:
|
@annadanchenko @goranjovic had the same issue possibly on develop yesterday, @goranjovic could you confirm? |
yes, it's in nightly too #7332 |
Tested Mac OS and Linux builds - fresh install, upgrade from current nightly, creating new account, restoring account from seed phrase, everything looks fine |
25ac147
to
99db611
Compare
Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>
99db611
to
d3f1b37
Compare
Summary:
This PR incorporates status-im/status-go#1352
Testing notes (optional):
Although this is a simple change, it's maybe a good idea to try it out on iOS and one desktop platform just to make sure.
status: ready