Opened 6 years ago
Closed 5 years ago
#14120 closed defect (fixed)
Remove methods deprecated during gsoc-core
Reported by: | Don-vip | Owned by: | michael2402 |
---|---|---|---|
Priority: | normal | Milestone: | 18.06 |
Component: | Core | Version: | |
Keywords: | gsoc-core | Cc: | bastiK |
Description (last modified by )
Deprecated features from gsoc-core to remove, which are still used in core or plugins:
LatLon.heading()
:used in 5 plugins: improveway, iodb, nanolog, photoadjust, rex, SimplifyAreaPreferences.getColor()
:used in MapCSSParser + plugins: utilsplugin2, commandLine, routing, graphviewLayer.isChanged()
:used in MapView, GpxLayer, AbstractTileSourceLayer + plugin osminspectorImageryLayer.getDx()/getDy()/setOffset()/displace()
: used in ApplyOffsetAction, ImageryLayer + 4 plugins: commandLine, imageryadjust, iodb, irsrectify
Attachments (0)
Change History (70)
comment:1 Changed 6 years ago by
Milestone: | 16.12 → 17.01 |
---|
comment:2 Changed 6 years ago by
Milestone: | 17.01 → 17.03 |
---|
comment:7 Changed 6 years ago by
@michael2402:
Can you please give fixing the deprecations highest priority? We need to get a clean state, before we start other deprecations.
comment:8 Changed 6 years ago by
Status: | new → assigned |
---|
comment:9 Changed 6 years ago by
Type: | enhancement → defect |
---|
comment:10 Changed 6 years ago by
14.04 is due in two weeks, can you please fix the core deprecation warnings before that?
comment:11 Changed 6 years ago by
I think I have updated all the plugins now. But it seems we still have a bug in core:
- #13503: IAE: Cannot paint layer, it is not registered (TMSLayer)
- #13604: IAE: Cannot paint layer, it is not registered (OsmDataLayer)
- #13886: IAE: Cannot paint layer, it is not registered (ValidatorLayer)
- #14234: IAE: Cannot paint layer, it is not registered (GeoImageLayer)
- #13589: IAE: Cannot paint layer, it is not registered (ElevationProfileLayer)
comment:12 Changed 6 years ago by
comment:16 Changed 6 years ago by
I already saw that. Some problem with the test framework removing a layer twice, i'm debugging it but tests always take some time to run ;-)
comment:22 Changed 6 years ago by
I found the problem:
The map view handles the layer lifecycle. If tests create multiple map views for the same layer, each map view destroys the layers during test exit - so we have several destroy() calls. I removed one of the places where MapViewMock is used. We could completely remove that Mock, I think.
comment:24 Changed 6 years ago by
Description: | modified (diff) |
---|---|
Milestone: | 17.04 → 17.05 |
comment:27 follow-up: 29 Changed 6 years ago by
I've seen them. The most important thing for me is getting the warnings safely away until next release. They are caused by a selection change event that was used to trigger a map view repaint in most cases. There is a reason why someone added those calls back then, so I'm going trough the UI code and fixing them one by one (or check if it is already fixed by some other mechanism that triggers a repaint). And I'm fixing some other repaint/layout related issues on the way.
The AlignInLineActionTest(s) are caused by the selection not being sorted for a single addSelection(). I tried around a bit and it seems the LinkedHashSet order gets lost in the streams, even when using a LinkedHashSet as collector. Have to look into it more. I was not aware that the selection order is used that much.
If users select the points, multiple addSelection calls will be issued. So for now, this is only a problem if a plugin uses this functionality.
I'll probably set up travis/git again next week, so that I will be able to run the tests in background more easily.
After the selection stuff is done, I already have a patch for the GPX layer that removes the update counting code. That way, we can get rid of the isChanged() methods and rely on layer invalidation events in the future. This should allow us to remove most of the MapView.repaint() calls in code.
comment:29 Changed 6 years ago by
Replying to michael2402:
I was not aware that the selection order is used that much.
...
So for now, this is only a problem if a plugin uses this functionality.
(Just in case you didn't know: When creating turn restrictions with the turnrestrictions plugin, the selection order is required as the roles are set automatically depending on the selction order.)
comment:30 Changed 6 years ago by
Cc: | bastiK added |
---|
comment:35 Changed 6 years ago by
Only one test to fix and we won't get spammed by Jenkins at each build anymore: MapCSSRendererTest.testRender[order]
. After that I expect the build to be stable a longer time. A test failing for 91 builds is a lot of noise :(
comment:36 Changed 6 years ago by
I did not know you get a mail for every test run...
But now they are all fixed. There a still a a few deprecations to track down, but then everything should be stabilized for the next release.
comment:37 Changed 6 years ago by
Milestone: | 17.05 → 17.06 |
---|
I don't want to remove the remaining selection changed calls now since I did not fully test if/where they are required.
comment:39 Changed 6 years ago by
Milestone: | 17.06 → 17.07 |
---|
comment:40 Changed 6 years ago by
Milestone: | 17.07 → 17.08 |
---|
comment:41 Changed 6 years ago by
Milestone: | 17.08 → 17.09 |
---|
comment:43 follow-up: 47 Changed 6 years ago by
I'll have a look at it.
The line can simply be removed in theory, the hard part is to check if all UI components are actually redrawn correctly if commands are executed.
comment:45 Changed 6 years ago by
Milestone: | 17.09 → 17.10 |
---|
comment:47 follow-up: 49 Changed 6 years ago by
Replying to michael2402:
The line can simply be removed in theory, the hard part is to check if all UI components are actually redrawn correctly if commands are executed.
Looks like it is working. Why is the private method in DataSet
named fireDeprecatedSelectionChange
?
comment:48 Changed 6 years ago by
Description: | modified (diff) |
---|
comment:49 Changed 6 years ago by
Replying to bastiK:
Why is the private method in
DataSet
namedfireDeprecatedSelectionChange
?
Because I wanted to deprecated the old selection listener mechanism first - but it was used in too many places so that I could not do this on the inital patch. And then I forgot about the method name...
See #13467
comment:50 Changed 6 years ago by
Milestone: | 17.10 → 17.11 |
---|
comment:51 Changed 6 years ago by
Milestone: | 17.11 → 17.12 |
---|
comment:52 Changed 6 years ago by
@team: I'm not sure how to update last plugins regarding deprecation of heading()/getDx()/getDy()
I'd like to get some help.
comment:53 Changed 5 years ago by
Milestone: | 17.12 → 18.01 |
---|
comment:55 Changed 5 years ago by
Milestone: | 18.02 → 18.03 |
---|
comment:56 Changed 5 years ago by
Milestone: | 18.03 → 18.04 |
---|
comment:57 Changed 5 years ago by
Milestone: | 18.04 → 18.05 |
---|
comment:58 Changed 5 years ago by
Milestone: | 18.05 → 18.06 |
---|
comment:62 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:65 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:66 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:67 Changed 5 years ago by
Description: | modified (diff) |
---|
I don't have much time right now and won't have until March 31. I would like to re-schedule this one for milestone 17.4 (I just put it to 17.3, since there is no 17.4 yet).