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

Next feature release #8615

Closed
moving-bits opened this issue Jul 12, 2020 · 46 comments
Closed

Next feature release #8615

moving-bits opened this issue Jul 12, 2020 · 46 comments
Assignees
Labels
Feedback required Issue requires feedback of the author or another user

Comments

@moving-bits
Copy link
Member

Let's open this issue to discuss which things should go into the next feature release, and which should stay in the queue for now.

@moving-bits moving-bits added the Feedback required Issue requires feedback of the author or another user label Jul 12, 2020
@moving-bits
Copy link
Member Author

Looking at the current PR queue the following things should/can go into the next feature release IMHO:

The other open PR I would like to postpone for now:

For #8611 it depends, see below.

Also, I would like to finish my work on the "fast scroll" behavior. Guessing from the feedback it seems to work reasonably well now for the cache list, so I can start extracting it to a reusable feature to be applied to other lists in c:geo which have fastscroll enabled.


One thing that we should discuss is how to proceed with "online maps onboarding":

We should decide whether we want to keep the implementation of #8183 or not:

  • if not: we should remove it before releasing
  • if yes: it should be accompanied by the upcoming "map download" feature

My proposal would be:

This would remove the ability to open locally stored .map files, but would offer c:geo much less frequently when tapping on local files, avoiding the effects described in #8467.


@Lineflyer
What do you think?

@Lineflyer
Copy link
Member

Thanks @moving-bits for the good summary.

Regarding the maps onboarding feature:
I did meanwhile understand the reason of the "cgeo file manager", however I can still not believe (while I have no technical background to judge) that there is no way to limit this intent somehow. Newest finding: If I look into my contacts it offers to send a message to my contact using c:geo file manager.
Let me say: I would prefer, that this is removed just to avoid the clutter on users device and the mass amount of questions and support cases I do expect based on that. (Will comment in more detail on #8467).
That being said:
I would also vote for the second approach, but as I always prefer time for testing I would additionally suggest to remove the first approach (or at least the parts involving this c:geo file manager) and try the secons approach only after merging branches.

For the list of PRs above I agree for most, only regarding #8589 I am a bit undecided as I did not yet understand if this is just a bugfix or also giving up some existing parsings.

@moving-bits
Copy link
Member Author

Ok, let's do it that way. I merged #8612 and prepared two new PR:

PR #8611 is postponed for now, as it's needed only for the new "map downloader" currently (and might be used for #8603 as well).

As soon as we have branched I can merge the new "map downloader" to master so that you have a chance to test it. Then we can still decide whether to integrate it into the upcoming release or into a later release.

@Lineflyer
Copy link
Member

As soon as we have branched I can merge the new "map downloader" to master so that you have a chance to test it. Then we can still decide whether to integrate it into the upcoming release or into a later release

Sound like a good plan! :)

@moving-bits
Copy link
Member Author

@Lineflyer: Anything that needs to be done for the next release regarding #8517?

@moving-bits
Copy link
Member Author

@Lineflyer: How about #8477 (comment)?

@Lineflyer
Copy link
Member

IMHO the #8477 comment was solved or not. I will test a fresh installation and compare, because now I already played around with the settings.

@Lineflyer
Copy link
Member

@Lineflyer: Anything that needs to be done for the next release regarding #8517?

I will doublechek as time permits. Just merged the german translations and look over it next days.

Lets keep the nightlies/master stable next days to see if its working smooth. Then we take action.

@moving-bits
Copy link
Member Author

IMHO the #8477 comment was solved or not. I will test a fresh installation and compare, because now I already played around with the settings.

I have not changed the way "history track line" gets drawn yet - it's still a "broad" line (standard: grey) with a smaller white line on top. As this is the only line drawn in this way this might be a reason to change it to the way all other lines are drawn - just as a plain line. OTOH this would give a different visual effect.

@moving-bits
Copy link
Member Author

@Lineflyer: Anything that needs to be done for the next release regarding #8517?

I will doublechek as time permits. Just merged the german translations and look over it next days.

Thanks.

Lets keep the nightlies/master stable next days to see if its working smooth. Then we take action.

Yes. I have merged the "fastscroll" and "disable file manager" PR already, so other than the two issues mentioned above (#8517 / #8477 (comment)) there's currently nothing new planned for master branch from my side.

(BTW: #8477 itself is resolved, just the linked comment is TBD.)

@Lineflyer
Copy link
Member

@moving-bits Do you have an overview whether there are any regression issues in our lists (meaning regressions compared to current release)? I will also do a scan of our issues next days.

@Lineflyer
Copy link
Member

Oh yes: #8457

@moving-bits
Copy link
Member Author

Oh yes: #8457

This should not happen for now on devices running up to Android 10, as I'd enabled the official mitigation for it (android:requestLegacyExternalStorage="true" in AndroidManifest.xml).

But it will not work on the upcoming Android 11 devices, as Android enforces the new rules then. For that we'll have to rewrite our storage framework.

@Lineflyer
Copy link
Member

Lineflyer commented Jul 14, 2020

This should not happen for now on devices running up to Android 10, as I'd enabled the official mitigation for it (android:requestLegacyExternalStorage="true" in AndroidManifest.xml).

I missed that, so I will give it another test run.

@Lineflyer
Copy link
Member

Seems we look rather good now on our way to a new beta version.
Only #8597 is left over to be reviewed and merged if OK.

Based on my personal timing I would suggest two different options to decide upon:

  1. Keep master stable for a week (suggest until 24.07. as I am on vacation from 17.07. - 24.07. ;) ). If no big problems arise, merge branches and continue publish a beta version
  2. Merge branches before 17.07. and publish a beta. This will allow collecting feedback a week earlier and not block further process on master

I would prefer option 2, the only downside is, that I might probably unable to help and answer support mail if there is any serious problem on beta. But as I do not expect that, I would still prefer the faster approach.

@moving-bits
Copy link
Member Author

Both options would be fine for me, so go ahead with option 2 if you like.

Can you give me a short feedback on #8477 (comment)? That's about if we want to change history track line to a regular non-shadowed line like all the other lines are. IMHO now would be a good timing for that with all that formatting changes for the map lines. If ok from your side I would prepare this as small PR for upcoming feature release. - Except if you say better not to change the shadowing.

@Lineflyer
Copy link
Member

For #8457 I tested the workaround on Android 10 and its working.

So @cgeo/team please double check if there is anything to do on master before merging branches (potentially tomorrow).
What about #8597?

@moving-bits
Copy link
Member Author

#8597 is merged meanwhile.

I'm unsure about #8646:

No idea for #8632 yet, but for a beta we may continue for now.

#8647 should be included - avoids painting OSM history track line in a "dotted style".

@Lineflyer
Copy link
Member

I left yesterday for vacation.
This means I could still merge branchs, but beta version only next week.

Just tell me, when you think everything is in.

@moving-bits
Copy link
Member Author

Without further information I cannot reproduce #8632 nor #8646, so I'm a bit lost. Are those two affecting other people as well, anything on support? Shall I implement mitigations (null checks avoiding the NPE)?

Other than that nothing urgent left on my radar currently (but I may have missed something), so IMHO branches could be merged any time now.

@Lineflyer
Copy link
Member

Ok, will do it and publish beta either after vacation or if I find time and technical possibility before. Will report back here.

@moving-bits
Copy link
Member Author

AFAIC we seem to have a pretty stable version now, after #8663 and #8664 having been fixed.
Only #8661 and #8670 left on my mind, but both non-critical, and both could be added during a beta phase, IMHO.

@Lineflyer
Copy link
Member

I will try to look on it this weekend when back from vacation.

@bekuno
Copy link
Member

bekuno commented Jul 24, 2020

I have to test the reverts for #7998. I hope that it is finished tomorrow.

@Lineflyer Lineflyer self-assigned this Jul 25, 2020
@Lineflyer
Copy link
Member

Lineflyer commented Jul 25, 2020

@moving-bits
Copy link
Member Author

I'm receiving an NPE in Routesort view when loading has not yet finished. Will create issue + PR for that.

@moving-bits
Copy link
Member Author

@Lineflyer
I've merged the open issues I'd been working on for master branch (including the fix for the NPE mentioned above), updated changelog accordingly, and added a warning info about Android 11 (due to #8457).
From my side "ready to go" except maybe reverting #7998.

@Lineflyer
Copy link
Member

I've merged the open issues I'd been working on for master branch (including the fix for the NPE mentioned above), updated changelog accordingly, and added a warning info about Android 11 (due to #8457).

Perfect. thanks.

From my side "ready to go" except maybe reverting #7998.

OK, I will merge branches now.
@bekuno For reverting #7998 please submit PR towards release.

@Lineflyer
Copy link
Member

Be aware: Branches have been merged

We do now have:
release = Currently equals master and base for upcoming beta version, any fixes for regressions should be merged here until release is ready
master = Free for new developments
cgeo-legacy= Contains status of previous release for reusing as a legacy version parallel to next release

I will try to build, test and publish beta tonight or tomorrow.
If possible on Google Play I will also try to bring the legacy version into the beta test.

@Lineflyer
Copy link
Member

@moving-bits
While smoke testing the possible RC/beta version I stumbled upon an empty changelog page in About c:geo.
While merging branches I exchanged the changelog content between changelog_master and changelog_release and hopefully did everything right.
The content of changelog_release was previously appended below the changelog_master on the "Changes" tab in About c:geo.
Can you please crosscheck whether I did something wrong?

@moving-bits
Copy link
Member Author

Thanks, @Lineflyer, for merging the branches and starting to prepare beta and legacy versions.
I will then start merging the PR currently being on hold into master over the next days, to get the queue freed up.

@moving-bits
Copy link
Member Author

While smoke testing the possible RC/beta version I stumbled upon an empty changelog page in About c:geo.
While merging branches I exchanged the changelog content between changelog_master and changelog_release and hopefully did everything right.
The content of changelog_release was previously appended below the changelog_master on the "Changes" tab in About c:geo.
Can you please crosscheck whether I did something wrong?

I'll look into it.

@moving-bits
Copy link
Member Author

While smoke testing the possible RC/beta version I stumbled upon an empty changelog page in About c:geo.
While merging branches I exchanged the changelog content between changelog_master and changelog_release and hopefully did everything right.
The content of changelog_release was previously appended below the changelog_master on the "Changes" tab in About c:geo.
Can you please crosscheck whether I did something wrong?

I'll look into it.

Does work here for both master and legacy branch. (Tested with emulator, API 29)

@Lineflyer
Copy link
Member

Release branch is the problem.
I tested on an Android 5.0.1 device.

@moving-bits
Copy link
Member Author

Release branch is the problem.
I tested on an Android 5.0.1 device.

Ok, I think I've found the culprit: The \n in the middle of the warning message regarding Android 11 confuses Android 5 (although it works on API 29). I'll replace it by " - " (on release branch).

@Lineflyer
Copy link
Member

Ok, I think I've found the culprit: The \n in the middle of the warning message regarding Android 11 confuses Android 5 (although it works on API 29). I'll replace it by " - " (on release branch).

Did you already do that?
Afterwards I would push the beta versions.

@moving-bits
Copy link
Member Author

Yes, had changed this yesterday on release branch.

@Lineflyer
Copy link
Member

OK, I uploaded a new multiple APK beta version:
2020.07.26-RC / 2020.07.24-RC-legacy

It consists of our normal RC from release branch and the legacy version from cgeo-legacy branch.
Beta users on API 16-20 should receive the legacy version, API 21+ should receive the normal RC.

I will update versions on status.cgeo.org in a few minutes so that we can see user counts.

@Lineflyer
Copy link
Member

I just triggered some proofreadings on crowdin to get more translations in.
Please kindly wait with further updates to master until #8677 is tested and merged. Then I will again fast-forward release to the head of master to have these translations included in the upcoming release.

@Lineflyer
Copy link
Member

I just triggered some proofreadings on crowdin to get more translations in.
Please kindly wait with further updates to master until #8677 is tested and merged. Then I will again fast-forward release to the head of master to have these translations included in the upcoming release.

DONE!

@moving-bits
Copy link
Member Author

Next thing I would like to merge is #8611, as this is the base for two other PR, but CI constantly fails with an error message I do not know the reason for, see my note in #8611. (Something like "no tests found" - looks to me like a problem with the CI.)

Any ideas?

@moving-bits
Copy link
Member Author

@Lineflyer
Have you had a look at #8643 from qs & support perspective? (I'm not that deep into proceedings of personal note replication.) If ok, we can merge, but we need to squash the PR first.

@Lineflyer
Copy link
Member

Have you had a look at #8643 from qs & support perspective? (I'm not that deep into proceedings of personal note replication.) If ok, we can merge, but we need to squash the PR first.

Well, for sure this problem should be solved.
But I am not sure, whether this PR fixes it, as according to PR description it handles merging between provider and local, whereas my experience shows, that the duplication happens while the cache is saved and without any server interaction.
But lets test it, if the code looks good to you. We do now have most possible time on master.

@Lineflyer
Copy link
Member

Next thing I would like to merge is #8611,

I guess the CI failure is somehow related to your PR as it runs smoothly for other PRs.

@Lineflyer
Copy link
Member

Lineflyer commented Aug 4, 2020

Ready to go:

  • Check/Insert correct version name as headline in changelog-release.xml
  • Build release on CI server
  • Perform smoke test with resulting cgeo-release.apk before continuing
  • Upload and publish cgeo-release.apk to Google Play
  • Tag the release on Github
  • Publish the release on Github including upload of cgeo-release.apk to Github
  • Publish release on F-Droid
  • Trigger notification to status.cgeo.org via CI server once the release is available on Google Play
  • Post release info to Facebook
  • Post release info to Twitter

@fm-sys
Copy link
Member

fm-sys commented Aug 26, 2020

Let's close this, since this old release is finished and we are currently preparing a new one...

@fm-sys fm-sys closed this as completed Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feedback required Issue requires feedback of the author or another user
Projects
None yet
Development

No branches or pull requests

4 participants